fix(rees): stop dropping added lines whose content starts with ++#2398
fix(rees): stop dropping added lines whose content starts with ++#2398glorydavid03023 wants to merge 1 commit into
Conversation
collectAddedLines and filesHaveAddedLines skipped any patch body line starting with +++ or ---. That guard is only meant for unified-diff file headers (`+++ b/file`, `--- a/file`), which always have the marker run followed by a space. Git renders an added line whose content is `++x` as `+` + `++x` = `+++x`, so it was mistaken for a header: the line was dropped from addedLines, newLine was not advanced (mis-numbering every following added line), and filesHaveAddedLines returned false for a PR whose only addition starts with ++. addedLines feeds analyzers such as secret-scan, so a secret on such a line was silently skipped. Track whether the parser is inside a hunk instead: the diff preamble only precedes the first @@, and inside a hunk the first character is always the +/-/space op, so `+++x` is correctly read as an addition. This fixes the whole class (any content beginning with ++/--, with or without a following space). Adds a regression test. No issue because issue creation is restricted on this repo; this is a small, self-evident correctness fix in a pure diff parser with no schema or API change.
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - reject/close recommendedReview updated: 2026-07-02 05:07:39 UTC
🛑 Suggested Action - Reject/Close
Review summary Blockers
Nits — 3 non-blocking
Why this is blocked
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
JSONbored
left a comment
There was a problem hiding this comment.
Failing review, please see gittensory orb review comments for blockers.
Summary
collectAddedLinesandfilesHaveAddedLinesinreview-enrichment/src/analysis-context.tsskipped any patch body line starting with+++or---. That guard is meant only for unified-diff file headers (+++ b/file,--- a/file), which always have the marker run followed by a space. Git renders an added line whose content is++xas++++x=+++x(no space), so it was mistaken for a header:++xadded line was dropped fromaddedLines, and becausenewLinewas not advanced for it, every following added line in the hunk was numbered one too low;filesHaveAddedLinesreturnedfalsefor a PR whose only addition starts with++;addedLinesfeeds analyzers such assecret-scan.ts, so a secret on an added line whose content starts with++was silently skipped.Fix: track whether the parser is inside a hunk. The diff preamble (
+++/---headers,diff/index/mode lines) only precedes the first@@; inside a hunk the first character is always the+/-/space op, so+++xis correctly read as an addition. This fixes the whole class (any content beginning with++/--, with or without a following space), not just one reproduction. Pure diff parser — no schema or API change.No issue because issue creation is restricted on this repo for outside accounts; this is a small, self-evident correctness fix.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typecheck(the rees packagetsc -p tsconfig.jsonbuild is clean)npm run test:coverage(N/A —review-enrichment/**is outside Codecov'ssrc/**/*.tsinclude; see note)npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
review-enrichment(rees) package, which has its own build + test. I rannpm --prefix review-enrichment run build(clean) andnode --test test/analysis-context.test.ts— 12/12 pass, including the added regression and all pre-existing cases (byte-cap, context-only, dependency scans).review-enrichment/**is outside Codecov'ssrc/**include, so it carries no patch-coverage obligation. I did not commitreview-enrichment/analyzer-metadata.jsonorapps/gittensory-ui/src/lib/rees-analyzers.ts:analysis-context.tsis not an analyzer, so it does not affect the analyzer registry those files are generated from (the localmetadata:checkstaleness is a pre-existing LF↔CRLF artifact that reproduces on unmodifiedmainand passes in CI's Linux checkout). I did not run the UI/workers/MCP steps (unrelated to this rees-only diff).Safety
UI Evidencesection below. — N/A: no visible UI change.Notes
review-enrichment/test/analysis-context.test.tsassertscollectAddedLinesreturns[[1, "++x"], [2, "const y = 1;"]]for the+++xpatch (recovered line + corrected numbering) and thatfilesHaveAddedLinesreturnstruefor a+++x-only patch.collectAddedLinesandfilesHaveAddedLineswere fixed identically (the deletion side---xis likewise no longer mistaken for a---header).