test(pr-followup): add PrFixType assertions to ingestion tests#380
Conversation
Add type field assertions to verify that each ingest function populates the correct PrFixType: - ingestCommentEvent → REVIEW_FEEDBACK - ingestReviewEvent → REVIEW_FEEDBACK - ingestCheckRunEvent → CI_FAILURE - ingestMergeStateEvent(DIRTY) → MERGE_CONFLICT - ingestMergeStateEvent(BEHIND) → OTHER This closes the test gap identified in #365.
The implementation fix (passing correct type to enqueuePrFixItem) was already merged to main before this branch was cut. This PR adds test assertions verifying the existing correct behavior: - ingestCommentEvent → REVIEW_FEEDBACK - ingestReviewEvent → REVIEW_FEEDBACK - ingestCheckRunEvent → CI_FAILURE - ingestMergeStateEvent(DIRTY) → MERGE_CONFLICT - ingestMergeStateEvent(BEHIND) → OTHER Addresses reviewer concern about missing implementation changes.
|
Addressing review feedback: The implementation fix (passing correct Updated the PR body to clarify this and changed "Fixes" to "Refs" since the implementation was already done on main. |
Superseded by a newer automated review for this pull request.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
PR PR 380 Review — test(pr-followup): add PrFixType assertions to ingestion tests
Summary
This PR adds type field assertions to the existing PR follow-up ingestion test suite (src/lib/pr-followup-ingestion.test.ts). Per the PR body, the implementation fix (correctly passing type to enqueuePrFixItem) was already merged to main; this PR adds only the test coverage that verifies that behavior.
Change-by-Change Analysis
The diff adds 6 assertions across 5 existing test cases and 1 new test case:
ingestCommentEvent(actionable path) — assertstype: "REVIEW_FEEDBACK". Confirmed:ingestCommentEventpasses"REVIEW_FEEDBACK"viaEnqueuePrFixInput.typein the source.ingestCommentEvent(needs_human path) — assertstype: "REVIEW_FEEDBACK". Same path; type is consistent.ingestReviewEvent— assertstype: "REVIEW_FEEDBACK". Confirmed:ingestReviewEventpasses"REVIEW_FEEDBACK"in the source.ingestCheckRunEvent(actionable path) — assertstype: "CI_FAILURE". Confirmed:ingestCheckRunEventpasses"CI_FAILURE".ingestMergeStateEvent(BEHIND) — assertstype: "OTHER". Confirmed: BEHIND maps to"OTHER"in the source.- New test:
ingestMergeStateEvent(DIRTY) — explicitly tests the DIRTY/merge-conflict path assertingtype: "MERGE_CONFLICT". Confirmed: DIRTY maps to"MERGE_CONFLICT"in the source.
Standards Compliance
- Code Standards: No issues. Test assertions use standard Vitest
expectpattern consistent with the rest of the file. - No commit of secrets: N/A — test-only change.
- Error handling: N/A — pure unit test additions.
- Validation: N/A — assertions validate already-produced values.
Evidence Provider Findings
No evidence providers are configured for this repository.
Tool Harness Findings
The test file was read in full. Key observations:
- The mock client (
makeClient()) correctly capturestypein thedataobject when callingprFixQueueItem.create, so assertions onclient.items[0].typeare valid. - The mock includes
$transactionwhich wraps the create call, matching the real call path. - The new
MERGE_CONFLICTtest follows the same pattern as adjacent tests and correctly exercises thedirtymerge state.
CI Check Results
All CI checks passed (Validate, Docker Build, review). This is authoritative evidence that lint, typecheck, and tests all succeed with the additions.
Linked Issue Fit
No linked issue context was provided in the corpus. The PR body references issue PR 365 as the motivation, and the implementation notes clarify the relationship between this test-only PR and the already-merged implementation fix.
Unknowns / Needs Verification
None. The implementation source was read and the type mappings are confirmed. CI confirms all 34 tests pass.
Recommendation
Approve. This is a straightforward test-coverage PR: it adds assertions that verify existing correct behavior, introduces a new test case for a previously untested path (DIRTY merge state → MERGE_CONFLICT), and all tests pass. No functional changes to production code.
Refs #365
Summary
Add
typefield assertions to the ingestion test suite to verify that each ingest function populates the correctPrFixTypevalue. This closes the test gap identified in issue #365.Implementation note
The implementation fix (passing correct
typetoenqueuePrFixItem) was already merged to main before this branch was cut. The current code insrc/lib/pr-followup-ingestion.tsalready passes:ingestCommentEvent→type: "REVIEW_FEEDBACK"ingestReviewEvent→type: "REVIEW_FEEDBACK"ingestCheckRunEvent→type: "CI_FAILURE"ingestMergeStateEvent(DIRTY)→type: "MERGE_CONFLICT"ingestMergeStateEvent(BEHIND)→type: "OTHER"This PR adds the test assertions that verify this existing correct behavior.
Tests added
ingestCommentEvent→ assertstype: "REVIEW_FEEDBACK"ingestReviewEvent→ assertstype: "REVIEW_FEEDBACK"ingestCheckRunEvent→ assertstype: "CI_FAILURE"ingestMergeStateEvent(DIRTY)→ assertstype: "MERGE_CONFLICT"(new test)ingestMergeStateEvent(BEHIND)→ assertstype: "OTHER"All 34 tests pass.
Worker: saffron-normal
Model: litellm/nvidia