fix: runs.retrieve now uses the output stored on the TaskRun table instead of deprecated attempts table #1853
Conversation
…stead of deprecated attempts table
|
WalkthroughThe PR updates the task run retrieval process in the API presenter by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant P as ApiRetrieveRunPresenter
participant DB as Database
C->>P: Call run retrieval API
P->>DB: Execute findRun with SELECT clause (including runTags)
DB-->>P: Return run data
P->>P: Simplify output retrieval and compute attemptCount
P-->>C: Return processed run details
sequenceDiagram
participant T as tagTestTask
participant TT as taggedTask
participant R as Task Registry
T->>TT: Trigger taggedTask with initial tags
TT->>TT: Check for required tags and wait
TT-->>T: Return initial tags in output
T->>R: Query currently executing tasks for validation
R-->>T: Confirm task status
T->>T: Verify final run details and tag output
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
references/hello-world/src/trigger/tags.ts (1)
49-105: Comprehensive tag verification test implementation.The
tagTestTaskprovides robust verification of the tagging system, including:
- Triggering a tagged task
- Verifying running tasks can be queried by tags
- Confirming that tags persist through task execution
A few suggestions to improve robustness:
Consider making the wait times configurable rather than hardcoded to make the test more resilient:
-export const tagTestTask = task({ +export const tagTestTask = task({ id: "tag-test", - run: async (payload, { ctx }) => { + run: async (payload: { waitForRunningMs?: number, waitForCompletionMs?: number } = {}, { ctx }) => { logger.info("Starting tag verification test"); // Trigger a task with initial tags const handle = await taggedTask.trigger( { waitSeconds: 3 }, { tags: ["test-tag-1", "test-tag-2"], } ); // Wait a moment to ensure the task is running - await setTimeout(3_000); + await setTimeout(payload.waitForRunningMs ?? 3_000); // Query for running tasks with our tags const runningTasks = await runs.list({ status: "EXECUTING", tag: ["test-tag-1", "test-tag-2"], }); // ... - await wait.for({ seconds: 10 }); + await wait.for({ seconds: payload.waitForCompletionMs ?? 10 });apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (1)
159-160: Updated attemptCount calculation to handle both engine types.The code now properly computes
attemptCountbased on the engine type, using eitherattempts.lengthfor "V1" orattemptNumberfor newer engines.Consider adding a brief comment explaining why this conditional logic exists to help future maintainers understand the different handling for V1 vs. newer engines:
// We're removing attempts from the API - attemptCount: - taskRun.engine === "V1" ? taskRun.attempts.length : taskRun.attemptNumber ?? 0, + // For V1 engine, count attempts from the attempts array + // For newer engines, use the stored attemptNumber field + attemptCount: + taskRun.engine === "V1" ? taskRun.attempts.length : taskRun.attemptNumber ?? 0,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts(4 hunks)references/hello-world/src/trigger/tags.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
references/hello-world/src/trigger/tags.ts (2)
apps/webapp/app/runEngine/services/batchTrigger.server.ts (1)
payload(652-682)apps/webapp/app/runEngine/services/triggerTask.server.ts (2)
payload(516-547)payload(549-559)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
references/hello-world/src/trigger/tags.ts (2)
1-3: Appropriate imports added to support new tagging functionality.The additional imports for
runsfrom the SDK,assert, andsetTimeoutare aligned with the new tag testing functionality being introduced in this file.
26-47: Well-structured task for tag verification.The
taggedTaskimplementation is clear and includes appropriate verification of expected tags, error handling, and a clean return value structure. The task is designed to be used in testing tag functionality.apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts (3)
55-55: Added runTags to common selection fields.The addition of
runTagstocommonRunSelectensures that tag information is properly included in run queries.
73-87: Improved database query structure with more specific field selection.Switching from
includetoselectwith specific field selection is a good improvement for performance and clarity. This change properly selects only the necessary fields from the database.
130-147: Simplified output retrieval directly from TaskRun.This change correctly implements the PR objective by using output stored directly on the TaskRun table instead of searching through the deprecated attempts table.
Summary by CodeRabbit
runTagsfor improved task run selection.