🎯 Nexus: Task Loop (Declarative Polling) Proposal#134
🎯 Nexus: Task Loop (Declarative Polling) Proposal#134thalesraymond wants to merge 1 commit intomainfrom
Conversation
- Adds proposal.md - Adds tasks.md - Adds design.md - Adds specs/task-runner/spec.md Co-authored-by: thalesraymond <32554150+thalesraymond@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @thalesraymond, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive proposal for implementing declarative polling loops within Nexus tasks. The primary goal is to provide a structured way for tasks to wait for external conditions to be met, thereby separating this "business logic waiting" from transient network errors or other retriable failures. This enhancement aims to improve task visibility, prevent the misuse of retry mechanisms for polling, and offer more explicit control over long-running conditional operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-thought-out proposal for declarative polling loops in tasks, effectively separating transient failures from business logic waiting. The design and specification documents clearly outline the motivation, goals, and requirements for this new feature. The approach of wrapping RetryingExecutionStrategy with LoopingExecutionStrategy is a sound design decision. My review comments focus on clarifying some aspects of the loop configuration and ensuring consistency between the design and specification documents.
| ## Risks / Trade-offs | ||
|
|
||
| - **Risk**: Infinite loops if `maxIterations` is not set or set too high. | ||
| - **Mitigation**: `maxIterations` should be mandatory or have a strict default (e.g., 100). |
There was a problem hiding this comment.
The design.md suggests that maxIterations should be mandatory or have a strict default (e.g., 100). However, the spec.md defines the default as 10. This discrepancy should be reconciled. A default of 10 might be too low for many real-world polling scenarios, potentially leading to tasks failing prematurely due to loop limits rather than actual business logic completion. Please align these documents or provide a justification for the chosen default.
| - **GIVEN** a `TaskLoopConfig` object | ||
| - **THEN** it SHALL support: | ||
| - `interval`: Delay in milliseconds between iterations. | ||
| - `maxIterations`: Maximum number of iterations (default: 10). |
There was a problem hiding this comment.
The default maxIterations is set to 10. While this prevents infinite loops, it might be too restrictive for common polling use cases where more iterations are expected. Consider increasing this default (e.g., to 100 as suggested in design.md) or making it mandatory to ensure users explicitly consider the loop bounds.
| - **THEN** it SHALL support: | ||
| - `interval`: Delay in milliseconds between iterations. | ||
| - `maxIterations`: Maximum number of iterations (default: 10). | ||
| - `until`: A predicate function `(context, result) => boolean` that determines if the loop should stop (return true) or continue (return false). |
There was a problem hiding this comment.
The until predicate is defined as returning a boolean. Given that TaskStep.run returns a Promise<TaskResult>, it's possible the result passed to until might be a resolved TaskResult or the predicate itself might need to perform asynchronous checks. Please clarify if the until function can be asynchronous (i.e., return Promise<boolean>) or if it must be synchronous.
|
|
||
| - **WHEN** the `until` predicate returns `false` | ||
| - **AND** the number of iterations reaches `maxIterations` | ||
| - **THEN** the task SHALL fail with a "Loop limit exceeded" error. |
There was a problem hiding this comment.
To improve debuggability, consider enhancing the "Loop limit exceeded" error message. It would be helpful to include the name of the task that failed and the maxIterations value that was reached. For example: "Task '[taskName]' exceeded maximum loop iterations (X)."
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Proposal for adding declarative polling loops to tasks. This feature allows users to define a task that repeats until a specific condition is met, separating transient failures (retries) from business logic waiting (loops).
PR created automatically by Jules for task 3706591880558235212 started by @thalesraymond