fix: describe both related-doc reasons and stop sending a full conventions file twice - #123
Merged
Merged
Conversation
…header Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ore merge) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
umm-actually re-reviewed at No new findings (3 tracked finding(s) across all runs). umm-actually · deepseek/deepseek-v4.1-flash |
…vert before merge)" This reverts commit cd6bfc0.
…rantee Priority docs are read under a token budget and skipped when missing, diff-excluded, or already sent in full by another channel, so the header no longer claims they are sent on every review. Ship-Check: pr-review · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Point each prompt-text contract at the code that honors it (diff headers, excluded-files trailer, non-finding filter, phase merge, doc reason strings). List every conventions full-copy case in one place, and state why the system-prompt order matters. Reuse conventionsRenderInFull in truncateConventions. Build the conventions, diff, and prior sections as tag/body/tag joins. Render the reason attribute on diff-only blocks too. The prompt output is unchanged for every input callers produce. Ship-Check: code-quality · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… header Ship-Check: triage · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reason quote - A diff-only file block keeps its reason attribute. - The diff and prior-findings sections are asserted as whole sections. - Conventions exactly at the cap render in full, matching conventionsRenderInFull. - Truncation, metadata and related-file tests assert whole sections, not fragments. - The related-docs header's quoted reason is checked against what readPriorityDocs sets. Ship-Check: test-audit · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The related-file scan did not receive the conventions path when the conventions section carried the whole file, so a JS/TS conventions file that imports a changed file was sent twice. Correct two prompt.ts comments: readPriorityDocs skips the conventions file by exclusion rather than never receiving it, and filterNonFindings covers most, not all, of the quoted non-finding phrases. Ship-Check: bug-check · claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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
The user prompt introduced every related doc with "Documentation that may describe changed code". Priority docs are included whether or not the diff touches them, so that header misdescribed them. The header now names both inclusion reasons and points at each block's
reasonattribute:The section stays one section. Each doc block already carries
reason="priority documentation"orreason="mentions …", so the header only needed to stop claiming a single reason for both.A second fix rides along. When the conventions section carries the whole conventions file, the orchestrator is meant to send no other full copy. The related-file scan was not told to skip it, so a JavaScript/TypeScript conventions file that imports a changed file was sent twice: in the conventions section and again as a related-file block. The scan now gets the conventions path as an exclusion in that case.
Changes
src/orchestrate.ts:findRelatedFilesexcludes the conventions file when the conventions section already carries it in full.src/__tests__/orchestrate.test.ts: two exactexcludePathsexpectations now include the conventions path. Two new tests cover the full-section case (the file is excluded and no related-file block carries it) and the truncated case (nothing is excluded, so a related-file block can still carry the full text).src/review/prompt.ts:filterNonFindings,annotateDiff,renderExcludedFilesNote,mergePhaseFindings, and the reason stringsreadPriorityDocsandfindRelatedDocsset. A comment onbuildSystemPrompt's section array says the order matters, because sections cite each other as "above" and "below".conventionsRenderInFull's doc lists every case where the conventions section defers to another copy of the file.truncateConventionscallsconventionsRenderInFullinstead of repeating its length check.renderFileBlockkeeps a file'sreasonon diff-only blocks too. No caller sets a reason on a diff-only file today, so output doesn't change.src/review/__tests__/prompt.test.ts:reason, the whole diff section, and conventions rendered in full at exactly the cap.src/context/__tests__/workspace.test.ts: a new test feeds the realreadPriorityDocsoutput intobuildUserPromptand asserts the header and the README block together, so renaming the priority-doc reason string on either side fails.Testing
tscare clean.max_related_docsdefault insrc/config.tsfrom 10 to 12 without updating README, and a later commit reverts it. On that run README was a priority doc and no doc was mention-matched (priorityDocPaths: README.md,mentionMatchedDocsCount: 0). The review flaggedsrc/config.ts:156with "Reconcile the max_related_docs default with README's documented value", citing README's input table, so the model read the priority doc under the new header. That run used an earlier draft of the header. It had the same opening line and the same section and block layout, and differed only in how it described the two reasons.🤖 Generated with Claude Code