test: add generated workflow reliability contract#77
Open
khaliqgant wants to merge 1 commit into
Open
Conversation
📝 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
Suggested change
| "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.
Comment on lines
+104
to
+109
| 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.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.