test: preserve default getTask mock for DUPLICATE recovery#2283
Conversation
Address CodeRabbit review: non-canonical getTask lookups should return a default current-task fixture instead of undefined, so recovery paths that re-fetch the subject task cannot NPE.
📝 WalkthroughWalkthroughDuplicate-resolution tests now keep non-canonical task lookups populated with default task objects while preserving canonical task responses, covering recovery re-fetch behavior in lineage finalization and subtask deletion scenarios. ChangesDuplicate lineage recovery tests
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR preserves current-task fixtures in DUPLICATE recovery tests. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (1): Last reviewed commit: "test: preserve default getTask mock for ..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts (1)
116-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer JSDoc-style comments for FNXC annotations.
As per coding guidelines, use JSDoc-style comments (
/** ... */) instead of standard block comments (/* ... */) forFNXCannotations.
packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts#L116-L120: Change the comment block opening to/**.packages/engine/src/__tests__/triage-split-into-subtasks-delete.test.ts#L168-L172: Change the comment block opening to/**.🤖 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 `@packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts` around lines 116 - 120, Update the FNXC annotation comments to use JSDoc-style openings instead of standard block-comment openings. Change the comment at packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts lines 116-120 and the corresponding comment at packages/engine/src/__tests__/triage-split-into-subtasks-delete.test.ts lines 168-172; preserve their existing text and formatting otherwise.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts`:
- Around line 116-120: Update the FNXC annotation comments to use JSDoc-style
openings instead of standard block-comment openings. Change the comment at
packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.ts lines
116-120 and the corresponding comment at
packages/engine/src/__tests__/triage-split-into-subtasks-delete.test.ts lines
168-172; preserve their existing text and formatting otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 963af846-a31d-4334-8f8e-857b55d81fdc
📒 Files selected for processing (2)
packages/engine/src/__tests__/triage-finalize-duplicate-lineage.test.tspackages/engine/src/__tests__/triage-split-into-subtasks-delete.test.ts
Summary
Follow-up to #2275 review feedback:
getTaskmocks for DUPLICATE recovery tests now return a default current-task fixture for non-canonical IDs instead ofundefined.Context
CodeRabbit on #2275 noted that returning
undefinedfor every id except the canonical task could break recovery if the subject task is re-fetched mid-path.Test plan
triage-split-into-subtasks-delete.test.tstriage-finalize-duplicate-lineage.test.tsSummary by CodeRabbit