Fix duplicate documentation comments across pushes and add synthetic evals - #35
Merged
Merged
Conversation
Address the repeated documentation threads reported in PR #28 at publication rather than assigning every finding in a document the same location identity. Composer grouping and existing code fingerprints retain their current semantics, so unrelated same-category findings and executable examples under docs/ remain separate. For Markdown, MDX, reStructuredText and text files, match the complete sanitized comment body on the same path and diff side. Store its content fingerprint before the inline size cap and read existing comments without that marker by their body. Changed wording, missing content and ambiguous truncated legacy comments remain eligible for posting instead of suppressing a potentially different issue. Use the validated publication anchor during duplicate detection, including anchors supplied only by the posting plan. Require a known anchor for prose matching. Adapt both PR #28 regression fixtures: preserve unchanged comment identity across a seven-line shift while explicitly keeping distinct documentation findings apart. Add coverage for code under docs/, existing markers, missing anchors, proximity, changed content and full-body identity beyond the display truncation boundary. Introduce evals/synthetics and make evals with 13 deterministic scenarios covering real diff parsing, publication, persisted markers, GitHub response parsing and duplicate detection through a scripted transport. Include these scenarios in the normal test suite and TypeScript checks; document their scope and conservative wording policy. Validation: pnpm test (1,390 tests across 63 files), make evals (13 scenarios), pnpm run check, pnpm build and git diff --check all passed.
|
🧞 Codegenie review failed (
|
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.
Unchanged documentation findings can open new GitHub threads after a push moves their anchors. For example, moving the same comment from line 81 to line 74 changes its hunk fingerprint and falls outside the existing five-line proximity check.
This replaces #28 with a publication-level fix. Identical sanitized comment content matches across anchors on the same prose file and diff side, while distinct findings retain their existing grouping identities. Thanks to @klaidliadon for identifying the issue and contributing the original reproduction and tests.
Behavior
.md,.mdx,.rstand.txtfiles by their complete sanitized body, normalizing line endings and surrounding whitespace.docs/.The original PR's shifted-hunk fixture now checks the publication result. Its second fixture deliberately expects distinct same-category documentation findings to remain separate, avoiding the original file-wide merging tradeoff.
Synthetic evals and regression coverage
Add
evals/synthetics/andmake evals: 13 deterministic scenarios exercise real diff parsing, publication, persisted markers, GitHub response parsing and duplicate detection through a scripted GitHub transport. They cover shifted/different anchors, posting-plan anchors, unrelated nearby and distant issues, paraphrased advice, existing comments, different files and capped bodies. No credentials, network requests or model inference are required.The synthetic suite also runs in
pnpm testand is included in TypeScript checks. Unit/pipeline regressions additionally cover prose extensions, absent anchors, code fingerprint behavior and the two adapted PR #28 fixtures.Validation
pnpm test: 1,390 tests passed across 63 files.make evals: all 13 scenarios passed.pnpm run check: TypeScript and workflow validation passed.pnpm buildandgit diff --check: passed.Deliberate limit
This is deterministic content matching, not semantic equivalence. Reworded findings and truncated old comments without a full-content fingerprint may still produce another thread. The tests explicitly preserve that conservative behavior rather than hiding potentially new information.