Uh oh!
There was an error while loading. Please reload this page.
test: add generated workflow reliability contract - #77
Conversation
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive test suite for generated workflow reliability contracts ( ChangesGenerated Workflow Reliability Contract Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/generated-workflow-reliability-contract.test.ts (1)
202-223: ⚡ Quick winMake the “without artifact rewrite” contract explicit with an
artifactWriterassertion.This test currently infers no rewrite from warning text. Add a writer spy and assert it was not called so the behavioral contract is directly enforced.
Suggested patch
+ const artifactWriter = vi.fn().mockResolvedValue(undefined); const result = await runWithAutoFix(localArtifactRequest( 'workflows/generated/reliability-malformed.ts', workflowContent(), ), { maxAttempts: 7, runSingleAttempt, + artifactWriter, workflowRepairer: vi.fn().mockRejectedValue(new Error( 'Workforce persona response must be structured JSON or include fenced TypeScript artifact and JSON metadata blocks.', )), classifyFailure: fakeClassification, debugWorkflowRun: guidedDebugger, }); expect(result.ok).toBe(true); expect(runSingleAttempt).toHaveBeenCalledTimes(2); + expect(artifactWriter).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/generated-workflow-reliability-contract.test.ts` around lines 202 - 223, The test infers "no artifact rewrite" from a warning string instead of asserting behavior; add an artifactWriter spy (e.g., artifactWriter = vi.fn()) and pass it into the runWithAutoFix options alongside runSingleAttempt and workflowRepairer, then assert that artifactWriter was not called (e.g., expect(artifactWriter).not.toHaveBeenCalled()) for the retry attempt(s) to make the contract explicit; reference the runWithAutoFix invocation, the runSingleAttempt mock, and the workflowRepairer mock when adding the artifactWriter and the new assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.trajectories/completed/2026-05/traj_nc2i7uwepync.json:
- Line 47: The projectId value currently contains a user-specific absolute path;
replace it with a portable identifier (e.g., a repo-relative path or logical
slug) by editing the "projectId" field in the JSON (located where "projectId":
"/Users/khaliqgant/Projects/AgentWorkforce/ricky-workflow-never-fail" appears)
and set it to a non-machine-specific value such as "ricky-workflow-never-fail"
or "./ricky-workflow-never-fail"; commit the change and ensure any generation
scripts write repo-relative or slug values instead of absolute local paths.
In `@test/generated-workflow-reliability-contract.test.ts`:
- Around line 104-109: The test currently dereferences result.artifact!
immediately after calling generate, which can hide generation validation errors;
before const artifact = result.artifact! add an explicit assertion that
generation succeeded (e.g., assert/expect that result.success/result.ok is true)
and, on failure, include the generation diagnostics or result message so the
test fails with clear validation info; update the block around generate(...) /
result to assert success before using artifact and then proceed to use artifact
and weakArtifact as before.
---
Nitpick comments:
In `@test/generated-workflow-reliability-contract.test.ts`:
- Around line 202-223: The test infers "no artifact rewrite" from a warning
string instead of asserting behavior; add an artifactWriter spy (e.g.,
artifactWriter = vi.fn()) and pass it into the runWithAutoFix options alongside
runSingleAttempt and workflowRepairer, then assert that artifactWriter was not
called (e.g., expect(artifactWriter).not.toHaveBeenCalled()) for the retry
attempt(s) to make the contract explicit; reference the runWithAutoFix
invocation, the runSingleAttempt mock, and the workflowRepairer mock when adding
the artifactWriter and the new assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ae7931c-3763-4652-a3b0-3e070b3a402c
📒 Files selected for processing (4)
.trajectories/completed/2026-05/traj_nc2i7uwepync.json.trajectories/completed/2026-05/traj_nc2i7uwepync.md.trajectories/index.jsontest/generated-workflow-reliability-contract.test.ts
| }, | ||
| "commits": [], | ||
| "filesChanged": [], | ||
| "projectId": "/Users/khaliqgant/Projects/AgentWorkforce/ricky-workflow-never-fail", |
There was a problem hiding this comment.
Avoid committing absolute local paths in trajectory metadata.
Line 47 includes a machine-specific absolute path (/Users/...), which leaks local user information and makes artifacts non-portable across environments. Prefer a repo-relative identifier (or logical project slug) in committed files.
Suggested patch
- "projectId": "/Users/khaliqgant/Projects/AgentWorkforce/ricky-workflow-never-fail",+ "projectId": "AgentWorkforce/ricky-workflow-never-fail",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "projectId": "/Users/khaliqgant/Projects/AgentWorkforce/ricky-workflow-never-fail", | |
| "projectId": "AgentWorkforce/ricky-workflow-never-fail", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.trajectories/completed/2026-05/traj_nc2i7uwepync.json at line 47, The
projectId value currently contains a user-specific absolute path; replace it
with a portable identifier (e.g., a repo-relative path or logical slug) by
editing the "projectId" field in the JSON (located where "projectId":
"/Users/khaliqgant/Projects/AgentWorkforce/ricky-workflow-never-fail" appears)
and set it to a non-machine-specific value such as "ricky-workflow-never-fail"
or "./ricky-workflow-never-fail"; commit the change and ensure any generation
scripts write repo-relative or slug values instead of absolute local paths.
| const result = generate({ | ||
| spec: implementationSpec, | ||
| artifactPath: 'workflows/generated/reliability-policy.ts', | ||
| }); | ||
| const artifact = result.artifact!; | ||
| const weakArtifact = { |
There was a problem hiding this comment.
Guard result.artifact behind a success assertion for clearer failures.
Line 108 dereferences result.artifact! without asserting generation succeeded first, which can surface a null-deref instead of the validation errors when generation regresses.
Suggested patch
const result = generate({
spec: implementationSpec,
artifactPath: 'workflows/generated/reliability-policy.ts',
});
- const artifact = result.artifact!;+ expect(result.success, result.validation.errors.join('\n')).toBe(true);+ expect(result.artifact).not.toBeNull();+ const artifact = result.artifact!;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/generated-workflow-reliability-contract.test.ts` around lines 104 - 109,
The test currently dereferences result.artifact! immediately after calling
generate, which can hide generation validation errors; before const artifact =
result.artifact! add an explicit assertion that generation succeeded (e.g.,
assert/expect that result.success/result.ok is true) and, on failure, include
the generation diagnostics or result message so the test fails with clear
validation info; update the block around generate(...) / result to assert
success before using artifact and then proceed to use artifact and weakArtifact
as before.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Related
Verification
Note: validation in this worktree used a temporary root node_modules symlink to the main Ricky checkout for dependency resolution; it was removed before commit.