fix(cli): forward analysis context to generateReview (#802 Phase 3 A2-fix-3)#871
Merged
Merged
Conversation
…802 Phase 3 A2-fix-3) Continue the Codex silent-skip cleanup after #865 (availableContexts) and #869 (availableDependencies). The plan layer's `buildExecutionPlan` already derives fileTypes, relatedADRs, and reviewMode (review-runner.mjs:212-217), but runReviewPlan never read them off the plan return and never forwarded them to generateReview. Impact (silent, no error) - fileTypes: generateReview's verifier file-phase coherence check fell back to "lenient when fileTypes not provided". On exec, phase mismatch checks were effectively disabled. - relatedADRs: ADR cross-references were never injected into the prompt on the exec path. Skill outputs lost the "ADR-001 says X" anchors that `river run` produced. - reviewMode: the diff-size-driven context budget preset was always the generateReview default rather than the calibrated 'tiny|medium|large' the plan layer determined. Fix - Pull `plan.fileTypes`, `plan.relatedADRs`, `plan.reviewMode` off the buildExecutionPlan return and pass them to generateReview alongside the existing diff/plan/phase/config. Optional chaining (`?? undefined`) keeps backward-compat: tests that mock buildExecutionPlan with a minimal shape continue to work. Tests - 2 new unit tests in tests/cli-review-plan.test.mjs cover the forward path and the undefined-fallback path. - runners/github-action/dist rebuilt with Node 22.22.2. Follows: #864 (A2-1), #865 (A2-fix-1), #869 (A2-fix-2). Remaining silent-skip risks - --plan replay context-snapshot drift (deferred to A2-3). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
River Reviewer
選択されたスキル (1)
スキップされたスキル (82)
優先度サマリー
スコア (参考値)結果(スコア): 100/100 内訳:
指摘No findings. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the runReviewPlan function to propagate analysis context fields—specifically fileTypes, relatedADRs, and reviewMode—from the execution plan to the generateReviewImpl call. This ensures that prompt enrichment and coherence checks have access to the necessary metadata derived during the planning phase. Additionally, new unit tests have been added to verify that these fields are correctly forwarded and remain undefined when omitted for backward compatibility. I have no feedback to provide as there were no review comments to assess.
Contributor
PlanGate Review
PlanGate review decision: pass
ポリシー: critical=fail / major=fail-if-required (warn) / minor=comment-only / info=skipped — spec: |
4 tasks
s977043
added a commit
that referenced
this pull request
May 22, 2026
…872) * docs(troubleshooting): close out silent-skip cleanup through v0.51.1 Codex-recommended docs follow-up after the v0.51.1 release. The existing troubleshooting page only documented the v0.51.0 availableContexts fix; v0.51.1 closed two more silent-skip failure modes (#869 availableDependencies, #871 fileTypes/relatedADRs/reviewMode) that were not yet reflected. What this adds - "Skill appears in skippedSkills with reason missing dependency": new section describing the v0.51.1 fix and the --dependency / RIVER_AVAILABLE_DEPENDENCIES / RIVER_DEPENDENCY_STUBS=1 knobs. - "Finding output looks generic / missing ADR references": new section explaining the three derived-context fields the exec path was silently dropping, and confirming the fix is automatic in v0.51.1+. - "Where the plan layer forwards each context": small reference table that maps each forwarded field to the PR / release that wired it. - "Residual gap: --plan replay does not yet run skills": explicit pointer to the remaining #802 A2-3 work so adopters know the boundary of v0.51.x. No code changes. Follows: #864 (A2-1), #865 (A2-fix-1), #869 (A2-fix-2), #871 (A2-fix-3). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * docs: link the #802 issue reference (Gemini PR #872 nit) Make the introductory mention of #802 a real GitHub link, matching the style of the residual-gap section. No content change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
4 tasks
s977043
added a commit
that referenced
this pull request
May 22, 2026
…A2-fix-4) (#877) The final remaining silent-skip identified by the multi-perspective session review (Codex + Gemini): runReviewPlan was hard-coding riskMap: undefined when calling buildExecutionPlan, so the riskAssessment that buildExecutionPlan would have produced never appeared on the artifact, and never reached generateReview's prompt on the exec path. What changed - runReviewPlan loads the optional risk map via loadRiskMap (default path .river/risk-map.yaml). Missing file => null (no behaviour change). Malformed file => ReviewPlanError so it fails loudly instead of dropping the risk signal silently. - The loaded riskMap is forwarded to buildExecutionPlan, which already knew how to compute riskAssessment when given a non-null riskMap. - plan.riskAssessment is forwarded to generateReview alongside fileTypes / relatedADRs / reviewMode (the analysis-context bundle finalised in A2-fix-3). - loadRiskMapImpl is injectable so tests can drive the path without a fixture file on disk. Tests - 5 new unit tests cover happy path forwarding, null sentinel, load failure, generateReview hand-off, and the undefined-fallback when the plan omits riskAssessment. - runners/github-action/dist rebuilt with Node 22.22.2. Silent-skip cleanup status (post-A2-fix-4) - availableContexts (#865) - availableDependencies (#869) - fileTypes / relatedADRs / reviewMode (#871) - riskAssessment (this PR) - --plan replay context-snapshot drift is the only remaining item, deferred to A2-3. Multi-perspective review reference - Codex 86/100: "riskAssessment が runReviewPlan で riskMap: undefined 固定で実質ドロップ" 指摘 - Gemini 95/100: "riskAssessment の伝播漏れのみが唯一の明確な減点 対象" - 両者が独立に同じ点を 6 番目の silent-skip として指摘した。 Follows: #864 (A2-1), #865 (A2-fix-1), #869 (A2-fix-2), #871 (A2-fix-3). Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 22, 2026
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
Forward the analysis context that
buildExecutionPlanalready derives (fileTypes,relatedADRs,reviewMode) fromrunReviewPlanintogenerateReview. Continues the Codex silent-skip cleanup after #865 (availableContexts) and #869 (availableDependencies).Silent failures this closes
fileTypesrelatedADRsriver runproducedreviewModeChanges
plan.fileTypes / relatedADRs / reviewModeoff the buildExecutionPlan return and pass to generateReviewBehavior
?? undefinedensures these fields stay undefined and generateReview falls back to its existing defaults.Remaining silent-skip items
--planreplay context-snapshot drift (deferred to A2-3).Test plan
Follows: #864 (A2-1), #865 (A2-fix-1), #869 (A2-fix-2).
🤖 Generated with Claude Code