GitDesktop / Tickets / #125 feat(ai,pr-review): use author notes in manual reviews and fair-share review-context budgets

Anonymous - 2026-07-26

Originally posted by: cloudflare-workers-and-pages[bot]

Deploying gitdesktop with Cloudflare Pages

Latest commit: 052862f

Status: ✅ Deploy successful!

Preview URL: https://41ff78af.gitdesktop.pages.dev

Branch Preview URL: https://fix-own-comment-context-fair.gitdesktop.pages.dev

View logs

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

assigned_to: theBGuy
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Context for reviewers

Three sibling fixes to the AI-review context pipeline plus one feature, all downstream of a live defect on PR [#123]: a 6,177-char author context comment reached the reviewer as 1,501 chars (items 1–2 of 11) while the own-comments section had 18,000 chars budgeted — a fixed pre-budget cap the Review-context knob could never reach. This PR makes the per-item caps floors under fair-share allocation (own + external), pins the PR-opening comment under budget pressure, wires author "Notes for reviewers" into manually-run reviews (previously automated-only), and adds a per-run Ignore author notes toggle mirroring the two existing ignore rows. This very comment is the fix's field test: it exceeds the old 1,500-char cap, so if you can read item 16, the pipeline you are reviewing delivered it.

Numbered so you can cite or overturn them individually.

Caps became FLOORS, and floor×count can over-allocate the budget — by design. allocateBodyCaps guarantees every block its floor even when floors alone exceed the section budget; fitOwn/fit in truncate.ts remain the hard enforcement. The allocator's doc comment states this explicitly so the over-allocation isn't read as a bug.

A fitting block's cap is its max-min share, not its own length. A lone 100-char comment at budget 18,000 gets cap 18,000. Deliberate: it keeps n=1 = full budget and costs nothing (capBody never cuts a block under its cap).

The fitOwn pin reserves at most 35% of the cap for the opening comment. Rationale: the opening brief records design intent nothing supersedes, but the newest follow-ups carry the live dispositions — the pin must never crowd out the majority of the recency signal. The 0.35 is a judgment call, documented at the site.

When the newest-first walk keeps nothing, the newest block is head-sliced into the leftover. Without this, opener 510 + newest 5,560 at cap 6,000 delivered 510 chars and dropped the live dispositions (adversarial-review blocker, fixed in the same tree). Both degenerate shapes now spend the full cap; a 36,000-call invariant sweep (4 shapes × caps 1..9,000) found zero cap overruns and zero silent-empty results.

Degenerate empty output routes to dropped: true, so prompt.ts emits its explicit "[omitted to keep the current diff in context …]" marker instead of silently rendering nothing. The invariant: never text: "" with dropped: false out of the pressure branch.

The truncation marker is charged against the block's own cap (worst-case digit reserve), so a marked block can never exceed its allocation. The marker also flows into the distillation input deliberately — the ledger model should know its source was clipped.

The distill cache fingerprint gained #joinedLen. An in-place comment edit moves neither count nor newest-timestamp; the joined post-cap length does. Existing cached digests miss once and re-distill — self-healing, no schema bump.

DISTILL_BLOCK_CAP (own-distill.ts) was unreachable dead code and is now live again — blocks could never exceed 1,501 chars before this PR; now they can. Its 6,000 value was already sized for this. Not touched.

External findings pool ONE allocation across all reviewers — the budget is section-wide, not per-reviewer. Also: an inline finding whose body condenses to nothing (e.g. entirely a <details> block) is now dropped instead of rendering a bodiless - \path:line` —` line; review/comment kinds already behaved this way.

resolveExternalContext gained an optional trailing opts?: { budgetChars? }. Absent = the old 8K constant, so existing callers shift only in HOW the 8K is shared — which is the fix, not a regression. Both callers now thread the scaled profile, so the Review-context setting finally reaches the external section.

Notes security posture is unchanged. The author gate lives INSIDE resolveReviewerNotesContext (the PR [#91] invariant); neither new caller can supply, relax, or override author identity. The panel's query only drives row visibility — the run re-resolves independently, so no trust transfers from the UI. ignoreNotes can only suppress the lift, never widen it.

The run receives ignoreNotes && hasNotes, so a stale toggle can never keep suppressing notes after its row disappears (spec-review finding, taken).

The row hides on query failure while the run still resolves notes — deliberate asymmetry. Coupling the run to the panel query would let a transient UI fetch failure silently strip real grounding context from the prompt: a data-loss failure mode traded for a disclosure one. This mirrors the existing external-reviews row exactly. A degraded-state row on isError is the recorded follow-up shape.

queryKey omits kind in useReviewerNotes — a deliberate byte-mirror of useExternalReviews, which has the same omission with the same consequence (cached remote data can show the row on a same-number local PR). Fixing one without the other forks the house pattern; both together are a recorded follow-up.

Bitbucket: the notes marker comment posts but can never be lifted back (its external-reviews harvest returns an empty Vec by design), so the help guide scopes the claim to GitHub/GitLab and the provider !== "bitbucket" gate in reviews.ts is an optimization, not a behavior change.

Disclosures

Live E2E ran on a fixture PR (GitDesktopTesting [#7] with a posted notes marker comment): the row renders/gates/toggles/disables correctly, stacks with the prior-review row, and a real claude-cli/opus interactive review produced a "Recorded decisions" section honoring both note pre-empts — the deferred item was noted without re-flagging, and the pre-empted finding never appeared.

Notes are fetched twice per review (panel query + the run's own resolve) — same shape as the external path, covered by the existing forge-dispatch-dedup backlog entry.

biome ci false-fails on reviews.ts/runner.ts — both files are wholly CRLF on disk (autocrlf checkout); both proven format-clean EOL-agnostically (LF-normalized stdin through biome format is byte-identical). No line endings were converted; edits preserved each file's existing EOLs (numstat shows content-sized rows, no EOL flip).

Verification

pnpm build exit 0 · tsc -b --noEmit exit 0 · scoped biome ci clean on LF files · 97 scratch assertions + the 36,000-call sweep all pass · prompt.ts replaceAll invariants measured intact (10× "pull request", 7× the markdown-flavor phrase) · no NUL bytes · README/help/changelog synced (4 fragments).

Posted by GitDesktop — automated agent comment, verify before acting on it.

Related

Tickets: ~~#123~~
Tickets: #7
Tickets: ~~#91~~
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Summary

Two-pass fair-share allocation for the own-comments and external-findings prompt sections, plus wiring the author's "Notes for reviewers" (and a per-run ignore toggle) into manually started reviews. The allocator arithmetic is sound (I traced allocateBodyCaps' max-min walk, the capBody reserve, and fitOwn's pin/leftover cap accounting — none can overrun their cap), the notes path keeps the PR [#91] author gate untouched inside the resolver, and both resolveExternalContext call sites thread the scaled profile. Nothing blocking; two should-fixes where a cut still lands without the explicit note this PR introduces, and one README overclaim.

Correctness

should-fix — src/lib/ai/external-context.ts, formatExternalFindings (caps at ~line 220, render at ~228–243). The caps are allocated against the raw section budget, but the section is measured against that same budget only after rendering adds scaffolding: .replace(/\n/g, "\n ") adds 2 chars per newline in every capped body, plus -/- (review)/- (summary) prefixes, the `path:line` — tag, ### <reviewer>\n headers, and the line/block joiners. When the allocation saturates (the interesting case — e.g. a 20K CodeRabbit walkthrough + six 900-char Copilot inline findings at externalCharBudget 8,000: the walk hands the walkthrough the frozen 4,000-ish share and the inline findings their lengths, summing to exactly 8,000), the rendered blob is guaranteed to exceed the budget by the scaffolding, so fit in budgetReviewExtras head-slices it with a bare safeSlice — cutting the last rendered item mid-word, and, if the cut lands in the last 58 chars of a capBody note, leaving a dangling [comment truncated — 12 fragment. Note the own path does not have this bug: formatOwnComments' caller measures the rendered joinedLen against the same budget fitOwn enforces. Fix: reserve the scaffolding before allocating — compute scaffold = Σ(prefix_i + 1 + 2 × newlineCount(body_i)) + Σ_reviewers(header_r.length + 1) + 2 × (reviewers − 1) (newline counts on the uncapped body are a safe upper bound) and pass Math.max(0, budget - scaffold) to allocateBodyCaps; update the formatExternalFindings doc block (lines ~176–183) to say the fair share is over the budget net of rendered scaffolding, and leave EXTERNAL_FINDINGS_CHAR_BUDGET's defensive-default role as documented at resolveExternalContext.

should-fix — src/lib/ai/truncate.ts, fitOwn (single-block branch, the pin, and the newest-leftover slice). All three cuts still use bare safeSlice, so the explicit note pass 2 just added is lost exactly in the pressure regime. Two concrete cases from the shapes described in the PR itself: (1) opener 510 + newest 5,560 at cap 6,000 → restCap 5,488 → the newest block is head-sliced bare at 5,488, so the highest-signal comment trails off with no note; (2) when the oldest block was already capped by capBody, the pin re-slices it at Math.floor(cap * 0.35), which deletes the [comment truncated — N more characters …] line that pass 2 appended (or splits it, if the reserve lands inside those 58 chars) — the opening brief is cut twice and ends up looking complete. Fix: use capBody(text, cap) at all three sites (capBody charges the note against the same cap, so restCap, remaining and the ≤ cap invariant all still hold); since a block may already carry a pass-2 note at its tail, add a small exported stripTruncationNote(text) in truncate.ts beside TRUNCATION_NOTE_HEAD/TAIL and call it before re-marking so the counts don't nest, and extend the marker at prompt.ts:670 to cover this shape (it currently says only "middle comments are omitted first", which misdescribes both a single head-sliced block and a cut newest/opening comment).

Docs

should-fix — README.md (Notes for reviewers bullet, ~lines 285–291): "reach every review as first-class context … and the reviews you run yourself" overclaims. startReview gates the lift on context.provider !== "bitbucket" and target.kind === "remote", and per the recorded Bitbucket disposition the harvest returns nothing there — so a Bitbucket author's posted notes reach the automated (event-carried) review but never a manually started one, and a local PR's notes never reach a manual review either. src/features/help/content.ts:899 already scopes this correctly ("every review of a GitHub or GitLab PR"); mirror that wording in the README bullet so the two surfaces state the same claim.

Readability / consistency (nits)

src/lib/ai/own-context.ts:44 — OWN_BODY_CAP is now a floor while the sibling constants were renamed (INLINE_BODY_FLOOR/REVIEW_BODY_FLOOR/COMMENT_BODY_FLOOR); rename to OWN_BODY_FLOOR and update its own doc line ("When OWN_BODY_CAP × count > budget") and the allocateBodyCaps call.

src/lib/ai/truncate.ts:30–31 — TRUNCATION_NOTE_HEAD/TAIL are exported but consumed only inside truncate.ts (no TS test runner in this repo to import them); make them module-private, or export them together with the strip helper suggested above so the export has a real consumer.

src/lib/ai/truncate.ts:30 — the note reads "[comment truncated — …]" but is applied to inline findings and submitted review bodies too (external-context.ts:228, rendering - (review) …[comment truncated…]); "content truncated" reads correctly for all four kinds.

src/lib/ai/external-context.ts:142–144 + src/lib/ai/own-context.ts:44 — beyond the recorded floor-over-allocation decision, the floors are absolute while the section budget is knob-scaled: at small (0.5× → own 3,000 / external 4,000) or a 0.15× auto profile (floored at 1,000 per field), floor×count dominates and the fair-share walk never applies; consider scaling the floors by the same profile multiplier.

src/lib/stores/reviews.ts:378–387 and :926–940 (call site PrReviewPanel.tsx:264–274) — startReview/generate now end in three same-typed booleans; a transposed argument silently swaps which context is suppressed with no type error. An opts: { ignorePrior?, ignoreExternal?, ignoreNotes? } object would be checked; the QueuedRun fields and the drain call at :740 would move with it.

src/lib/ai/truncate.ts:309–313 — "that block is by construction the PR-opening context brief" isn't enforced: present[0] is just the oldest GD_COMMENT_ANCHOR-bearing comment (our own AI review bodies are filtered out, but an early refutation or thread reply is not), so soften to "the oldest comment, typically the opening brief" or detect the brief's marker.

Recorded decisions acknowledged (not re-raised)

ignoreNotes && hasNotes fails open — a transient panel-query failure hides the row and un-suppresses an active opt-out for that run; remains a recorded decision (notes 12/13), and I verified the run's independent resolve is what makes it fail open.

Notes fetched twice per review, plus the panel's forge_pr_view/forge_pr_external_reviews duplicating usePrDetails/useExternalReviews caches — recorded (disclosure 17, forge-dispatch-dedup backlog).

useReviewerNotes' queryKey omitting kind, byte-mirroring useExternalReviews — recorded (note 14).

Floor × count over-allocating the section budget with fit/fitOwn as hard enforcement — recorded (note 1); I confirmed both fitters do bound the output.

Posted by GitDesktop — AI output, verify before acting on it.

Related

Tickets: ~~#91~~
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI security audit · opus · automated

No genuine security issues in these changes — the new notes path only reads through resolveReviewerNotesContext, whose PR-author gate (pr.author vs it.author, case-folded, empty-author ⇒ drop) is present in src/lib/ai/notes-context.ts and cannot be supplied or relaxed by either new caller, the notes reach only the prompt (rendered as a framed ## Author's notes for reviewers section, never the DOM or any shell/file sink), and the budgeting rewrite (allocateBodyCaps/capBody/fitOwn) keeps safeSlice on every cut with fit/fitOwn still the hard enforcement.

Posted by GitDesktop — AI output, verify before acting on it.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Round 1 dispositions — 10 findings (2 reviewers), 9 accepted, 1 declined; all fixes land in the next push

Every finding was verified against the code before disposition. The two correctness should-fixes were both real — and adversarial re-review of our own fix for the first one refuted its initial shape, so what ships is stronger than what was asked for. Details:

External scaffolding overflow (should-fix) — accepted with an upgrade. Mechanism confirmed: caps were allocated against the raw budget while rendering adds prefixes/headers/indent, so a saturated section provably exceeded the budget and fit head-sliced it bare (your example's frozen share computes to 2,600 rather than ~4,000, but the mechanism is unaffected). The suggested reserve-before-allocating fix is in — but a shadow re-review of that fix proved a pass-1 reserve alone cannot close the class: when the per-kind floors bind, allocateBodyCaps lifts every cap back above any netted budget (measured: 15 × 2,000-char findings at budget 8,000 → rendered 10,842), and fit enforces min(remaining, budget) where remaining is unknowable at format time. So the disclosure guarantee moved to the enforcement point: fit's cut now goes through capBody, and any residual overflow ends in one well-formed [content truncated — N …] note — including a guard that drops a partial interior note the cut lands inside, so the dangling-fragment artifact can't be produced. The reserve stays for right-sized shares, now charged against provisional-cap newline counts (charging uncapped bodies measurably starved the section: a 40K walkthrough cost every inline finding 47% of its share).

fitOwn bare cuts in the pressure regime (should-fix) — accepted as a class. All three sites (single block, pin, newest-into-leftover) now cut via capBody; a trailing note on an already-cut block is stripped and its count folded in, so a double cut reports the cumulative true omission — never nested, never undercounted (asserted: reported + kept === the original length). The pin's re-attached note is indented into its list item, and the same treatment covers the two siblings the round didn't name: the raw-budget allocation in formatOwnComments (which also made the distill trigger fire on scaffolding) and capLedger's bare … (a twice-cut ledger now discloses cumulatively). Sweeps: ~1,500 fit caps + 54,000 fitOwn calls, zero half-written notes, zero cap overruns.

README "every review" overclaim (docs should-fix) — fixed. Scoped to "every review of a GitHub or GitLab PR", mirroring the in-app guide's wording; the underlying gate (remote && numeric && provider !== "bitbucket") is what both surfaces now describe.

Nits — 5 of 6 accepted: OWN_BODY_CAP → OWN_BODY_FLOOR (matches its siblings); TRUNCATION_NOTE_* de-exported (zero outside consumers, grep-verified); "comment truncated" → "content truncated" (the note renders on inline findings and review bodies too); the "by construction the PR-opening brief" comment softened to "typically" (an early anchored reply can be oldest); and startReview/generate's three trailing same-typed booleans replaced by a ReviewIgnoreOptions object threaded through the queued-run record (which stores the resolved flags, so a caller mutating its object post-enqueue can't alter a queued run) and the panel call site.

Declined: scaling the floors by the budget-profile multiplier. The floor is a minimum-readable-unit guarantee — 1,500 chars is roughly one meaningful comment, and halving it at small profiles halves that guarantee precisely for the constrained-context models that most need coherent context. Floor-dominated saturation is the documented degenerate regime with fitOwn's pin/drop, distillation, and fit-with-marked-cuts as the designed handlers (the fix in item 1 is what makes that regime disclose itself). If constrained profiles prove to need it, the right execution is scaling floors as a deliberate profile field with its own doc — not deriving them silently from the budget — and that's recorded as the reversal shape.

Copilot's capBody fallback finding — accepted with a correction; disposition on the thread.

Your "Recorded decisions acknowledged" section matched our context comment 1:1 — nothing there needed re-litigation, which is the intended effect of items 11–14/17 being on the record.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Two rounds of fixes landed cleanly: the scaffolding reserve is in on both sections, fit/fitOwn's three cut sites now go through capBody with note strip-and-restate, and the boolean tail became ReviewIgnoreOptions. I re-traced the allocator, capBody's reserve arithmetic (including the new priorOmitted/indent terms and the partial-note guard), and all four fitOwn branches — every path still returns ≤ cap and the cumulative counts don't nest. Nothing blocking; two should-fixes, both introduced by the fix hunks themselves.

Correctness

should-fix — src/lib/ai/truncate.ts:34 (TRUNCATION_NOTE_TAIL), as reached from truncate.ts:370 and src/lib/ai/own-context.ts:310. The note ends " more characters on the PR thread]", which is true for the two original consumers (own comments, external findings) but false for the two this push added. fit(input.priorText, priorBudget) (truncate.ts:475 → fit at :361–373 → capBody(text, cap) at :370) cuts the previous review's findings, which come from the persisted review-history record — a review the user never clicked Post has no copy on the PR thread at all. capLedger (own-context.ts:309–311, called at :272 and :284) cuts the distilled decision ledger, which is generation-model output that exists nowhere but the digest store. So an agentic reviewer — which has forge_pr_* comment tools attached (reviews.ts:604, mcpTools) — is told to go read a PR thread that does not contain the omitted text. Fix: make the tail source-neutral, e.g. const TRUNCATION_NOTE_TAIL = " more characters omitted]", and reword the constant's doc block at truncate.ts:28–34 — it currently justifies only "inline findings and submitted review bodies" and needs to name the prior-findings and distilled-ledger consumers too, since "content" already covers them. Knock-on: stripTruncationNote (:44–59) matches on these same two constants, and own-digest-store persists capped ledgers that may carry an old-format note (own-context.ts:285–292, ledger: capped); a cached ledger re-cut by fitOwn after the wording change would fail the strip and nest two notes. That self-heals on the next fingerprint change, but if you'd rather not carry it, prefix the fingerprint (e.g. v2#${survivors.length}#… at own-context.ts:266) so every cached digest misses once — and update the fingerprint doc on OwnCommentsDigest in own-digest-store.ts to match the new token shape if you do.

Docs

should-fix — src/features/help/content.ts:801. The Create-PR-dialog paragraph still reads "the notes post as its first conversation comment … before any automated review runs, and that review reads them as context" — the pre-change scoping. This push updated the AI-review section (:899–903) and the README bullet (README.md:287–292) to "every review of a GitHub or GitLab PR", leaving this as the one surviving copy of the old claim, exactly the stale-sibling case CLAUDE.md's "grep for the old wording" rule targets. Fix: extend the clause in place — "…before any automated review runs, and that review reads them as context; on GitHub and GitLab the code reviews and security audits you start yourself read them too — so a deliberate, documented decision isn't re-flagged" — keeping the GitHub/GitLab scoping so it doesn't overclaim for the Bitbucket dialog (whose marker comment posts but is never lifted back), and leaving the surrounding {{ai}} gate untouched since the paragraph is already inside it.

Nits

src/lib/ai/truncate.ts:413–415 — the comment justifies OWN_BLOCK_INDENT as "own blocks render their body under a two-space continuation indent", but this branch's own opening line (:405) says it also handles the distilled-ledger case, and a ledger arrives as a bare capLedger string with no - (author …) prefix and no indent (own-context.ts:272, :293); the indented note is harmless in markdown but the stated reason doesn't hold for that input — soften to "own comment blocks render under a two-space continuation indent (a distilled ledger doesn't, where the indent is merely inert)".

Resolved since last review

External scaffolding overflow — the reserve is now netted out before allocation (external-context.ts:251–267) and, more to the point, fit cuts through capBody (truncate.ts:370) with a partial-note guard (:161–170), so a saturated section ends in one well-formed note rather than a bare mid-word slice or a dangling [content truncated — 12 fragment.

fitOwn's three bare safeSlice cuts — single block (:416–417), pin (:437–439), and newest-into-leftover (:457–459) all now stripTruncationNote + capBody(..., omitted, OWN_BLOCK_INDENT), so a twice-cut block reports the cumulative omission under exactly one note instead of having its first note deleted; prompt.ts:670's marker was rewritten to describe that shape.

README "every review" overclaim — scoped to GitHub/GitLab, matching content.ts:899.

OWN_BODY_CAP → OWN_BODY_FLOOR (with its doc line), TRUNCATION_NOTE_* de-exported, "comment" → "content" in the note, the "by construction the PR-opening brief" comment softened at :394 and in the ReviewExtras.own doc, and the three-boolean tail replaced by ReviewIgnoreOptions — threaded through QueuedRun.opts, the drain at :759–762, useReviewRun's generate, and the single call site (PrReviewPanel.tsx:264, the only one in the tree).

Scaling the per-kind floors by the budget profile remains a recorded decision (round-1 item 5), with a deliberate profile field named as the reversal shape — not re-raised.

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Round 2 dispositions — 3 findings, all accepted and fixed in the next push

Source-false note tail (should-fix) — fixed, with the knock-on taken. TRUNCATION_NOTE_TAIL is now the source-neutral " more characters omitted]", and the constants' doc names all four consumers with the reason the wording must stay neutral (prior-review text is local history — a never-posted review has no thread copy; the ledger exists only in the digest store). The persisted-ledger knock-on you traced is real and was proven rather than assumed — a legacy-tail note yields omitted = 0 from the strip parser, so a cached ledger re-cut after the wording change really would stack a second note — so the fingerprint is now v2#… (every cached digest misses once and re-distills), and OwnCommentsDigest.fingerprint's doc records the versioning rule for future text-format changes. schemaVersion stays 1 deliberately: the record's shape is unchanged; the format tag now lives in the fingerprint. All four consumers re-verified through their real paths (formatOwnComments→fitOwn, formatExternalFindings, fit on a prior blob, capLedger→fitOwn) with cumulative counts asserted.

Stale sibling in the Create-PR help paragraph (docs should-fix) — fixed in place. The clause now reads "…and that review reads them as context; on GitHub and GitLab the code reviews and security audits you start yourself read them too". One deliberate scoping detail: the first half stays unscoped because a dialog-created PR's automated review reads notes on every forge — the pr-open event carries them directly, no lift involved — so only the manual-review clause needs the GitHub/GitLab qualifier. Swept content.ts, README, and the site for any other copy of the old claim: zero hits.

OWN_BLOCK_INDENT comment overclaim (nit) — fixed; it now distinguishes the per-comment block indent from the bare distilled-ledger input, where the two spaces are inert.

Verification on the batch: the full scratch suite re-ran green with the 9-char tail shift confirmed as pure arithmetic (all cumulative-count identities still exact), both invariant sweeps zero-violation, tsc -b and scoped biome clean, pnpm build green, prompt.ts replaceAll counts still 10/7 (untouched this round).

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Sound change; no blockers. The fair-share allocator, capBody's reserve arithmetic, and all four fitOwn branches still return ≤ cap on every path I traced (including the degenerate caps), and the round-2 fixes landed cleanly. One should-fix, created downstream of this diff rather than inside it: the larger blocks the new caps produce now hit a bare cut in own-distill.ts.

Correctness

should-fix — src/lib/ai/own-distill.ts:44–46 (DISTILL_BLOCK_CAP), as reached from src/lib/ai/own-context.ts:284 (distillOwnComments({ blocks: ownItems, … })) with the blocks this diff builds. Your note 8 records that the 6,000-char per-block cut was dead code before this PR (bodies were pre-capped at 1,500) and is now live — but it is still a bare safeSlice with no note, and it fires in the default configuration: resolveBudgetProfile returns scaledProfile(3) for every hosted/CLI frontier provider (context-budget.ts:194–206), so ownCharBudget = 18,000; two own comments of ~10,000 chars get caps = [9000, 9000] out of allocateBodyCaps, joinedLen > budget fires the distill (own-context.ts:254), and each ~9,000-char block is then cut to 6,000 silently. Two consequences: the ledger model loses 3,000 chars with no signal that anything was clipped (the exact failure mode this PR exists to fix), and because capBody's note sits at the end of the block, the cut eats the very marker note 6 says is fed into the distillation input deliberately. Fix: in own-distill.ts, import capBody alongside safeSlice from ./truncate and replace the map with capped = input.blocks.map((b) => capBody(b, DISTILL_BLOCK_CAP)) — drop the b.length > DISTILL_BLOCK_CAP conditional, capBody is already a no-op under the cap, and it charges the note against the cap so capped[i].length ≤ DISTILL_BLOCK_CAP still holds and the DISTILL_INPUT_CAP suffix arithmetic at :47–59 is untouched (keep safeSlice at :58 for the not-even-one-block fallback, or move that to capBody too for consistency). Reword DISTILL_BLOCK_CAP's doc at :5–8: "Head-kept: reviews front-load their blockers" → head-kept with an explicit truncation note, so the ledger model can tell the block was clipped. If you also want the reported count to stay cumulative for a block that already carries a per-comment note, export stripTruncationNote from truncate.ts and pair it exactly as the three fitOwn sites do — no doc change needed at truncate.ts:36, which already frames both helpers as the contract while the constants stay private.

Docs

nit — src/lib/ai/own-context.ts:262–267 and src/lib/ai/own-digest-store.ts:20–27: the stated reason for the v2 bump ("a stale ledger re-cut by fitOwn would stack a second note under the first") doesn't hold. A v1 note only ever sits at the end of a cached ledger (capLedger appends it there), and fitOwn reaches capBody only when only.length > cap (truncate.ts:413), so keep < text.length always and the head-cut removes that trailing note outright — or the partial-note guard at :165–173 drops its fragment. Nothing stacks. Keep the bump; it's still correct for a different reason: without it a cached v1 ledger carries the old, source-false "…on the PR thread" wording straight into the prompt, which is what round 2 set out to retire. Reword both comments to that reason (this corrects my own round-2 claim, so the two doc sites are the only change).

nit — site/src/data/capabilities.ts:425–430: README, help guide and changelog were synced but the site wasn't. The existing line ("Hand review context to the AI reviewer — per-branch notes deposited by your agent, posted with the PR") isn't falsified, so nothing is stale — but per CLAUDE.md's docs rule the broadened scope and the new opt-out have no site presence. One-line fix: extend that label to "… — read by every review, automated or one you start, with a per-run opt-out"; it's already ai: true, so no flag change, then cd site && pnpm build.

Resolved since last review

Source-false note tail — TRUNCATION_NOTE_TAIL is now " more characters omitted]" (truncate.ts:38) and the constants' doc at :28–36 names all four consumers with the reason the wording must stay source-neutral; stripTruncationNote/capBody still round-trip on the new tail.

Stale Create-PR help sibling — content.ts:801–803 now scopes the manual-review clause to GitHub/GitLab and leaves the automated-review half unscoped, which matches runner.ts's event-carried path; grep confirms no other copy of the old claim in content.ts, README, or the site.

OWN_BLOCK_INDENT comment overclaim — truncate.ts:417–421 now distinguishes the per-comment block indent from the bare distilled ledger.

Copilot's tiny-cap bare-slice flag is addressed in the current code (truncate.ts:147–156 — ellipsis for cap >= 1, with the reachability trace recorded at the branch); not worth re-raising.

Standing recorded decisions, unchanged and not re-flagged: floors not scaled by the budget profile (reversal shape = a deliberate profile field), and the duplicate forge harvests (panel query + run resolve, and useReviewerNotes overlapping useExternalReviews) parked on the forge-dispatch-dedup backlog.

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Round 3 dispositions — 3 findings, all accepted and fixed in the next push

DISTILL_BLOCK_CAP bare cut (should-fix) — fixed with full fidelity. Both distill cut sites (the per-block map and the not-even-one-block fallback) now go through stripTruncationNote → capBody(text, cap, priorOmitted), so a block that already carries a per-comment note is re-cut into exactly one note with the cumulative true count (asserted: reported + kept === the original 12,000 on your exact regression shape — a 12K comment capped to 9K by the 3× profile, then re-cut at 6K; the old slice left noteCount = 0). stripTruncationNote is now exported with own-distill as its named consumer — the round-1 export rationale, now satisfied; the note constants stay module-private. DISTILL_INPUT_CAP's suffix arithmetic untouched; capped[i].length ≤ DISTILL_BLOCK_CAP holds across the suite. One deliberate detail: the distill re-cut passes no indent — this text is ledger-model input, not rendered reviewer markdown, so column-0 notes are fine there.

v2-bump rationale (nit) — accepted; your correction of our round-2 claim was itself verified before we adopted it. The empirical trace agrees with you: a complete v1 note line shares the note head, so the partial-note guard drops it whole on a re-cut (measured: exactly one note, legacy count lost, never nested — our own round-2 test data already showed this and was mislabeled). Both doc sites now state the real reason the bump stays: an un-recut cached v1 ledger would serve the source-false "on the PR thread" claim into prompts indefinitely.

Site capability line (nit) — fixed. The notes capability now reads "…read by every review (automated or one you start) with a per-run opt-out"; cd site && pnpm build green (19 pages).

Verification on the batch: full scratch suite green including the new distill groups, both invariant sweeps zero-violation, tsc -b + scoped biome + pnpm build + site build all green.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Round-3 fixes verified against the code: both distill cut sites now go through stripTruncationNote → capBody, and the two v2 doc sites and the site capability line are corrected. No blockers; three nits, all on the new distill hunk and its doc collateral.

Nits

nit — src/lib/ai/own-distill.ts:51–54 (the capped map). Dropping the length conditional means every block is now rewritten, not just over-cap ones, and for two block shapes that rewrite is not a no-op: (a) a block that already carries a per-comment note but fits under 6,000 has its note stripped and re-attached at column 0, losing the two-space indent formatOwnComments gave it (own-context.ts:184–185 runs capBody then .replace(/\n/g, "\n ")), and gets trimmed further whenever b.length is within ~50 chars of the cap; (b) stripTruncationNote matches on text shape, so a block whose last line merely quotes the note format — routine in this repo, where review comments discuss it — is stripped and re-emitted with a "N more characters omitted" claim on a block nothing was omitted from, feeding the ledger model a false cut. Not disputing the column-0 choice for blocks the distill genuinely re-cuts (recorded decision); this is about the blocks it doesn't need to touch. Fix: put the length guard back around the pair — const capped = input.blocks.map((b) => { if (b.length <= DISTILL_BLOCK_CAP) return b; const { text, omitted } = stripTruncationNote(b); return capBody(text, DISTILL_BLOCK_CAP, omitted); }); — byte-identical for over-cap blocks (cumulative fold intact, capped[i].length ≤ DISTILL_BLOCK_CAP still holds, newestAlone/DISTILL_INPUT_CAP arithmetic untouched). Then delete the now-wrong sentence at :49–50 ("capBody is a no-op for a block that already fits and carries no note, so it replaces the old length conditional outright") and reword the DISTILL_BLOCK_CAP doc's cumulative-count clause at :9–11 to "a block that exceeds this cap may already carry a note from its own per-comment cap — the count restated here is CUMULATIVE across both cuts".

nit — src/lib/ai/own-distill.ts:63–71 (newestAlone + its comment). The branch is unreachable: capBody guarantees capped[i].length ≤ DISTILL_BLOCK_CAP (6,000) and DISTILL_INPUT_CAP is 48,000, so the first loop iteration (cost = capped[last].length, no joiner) always fits and keptCount ≥ 1. Keeping it as defense is fine, but the comment reads as a live path — append "unreachable while DISTILL_BLOCK_CAP < DISTILL_INPUT_CAP; kept so the fallback is correct if the constants ever converge."

nit — src/lib/ai/truncate.ts:48–49, stripTruncationNote's doc: "The note may be indented, since both callers render bodies with a \n continuation indent" is stale now the function is exported and own-distill calls it (with no indent) on already-rendered blocks. Reword to "The note may be indented: the own-comments and external-findings renderers emit bodies under a \n continuation indent" — the constants' doc at :36–39 already names own-distill as the third consumer, so no change needed there.

Resolved since last review

DISTILL_BLOCK_CAP bare cut — own-distill.ts:51–54 and the keptCount === 0 fallback at :66–69 both go through stripTruncationNote → capBody, so a twice-cut block reports one cumulative count; stripTruncationNote is exported (truncate.ts:51) and the constants' doc records why the pair stays the contract.

v2 bump rationale — own-context.ts:263–271 and own-digest-store.ts:18–27 now state the un-recut-cached-ledger reason. I re-verified the new claim: a v1 note is the ledger's final line, so any head cut either removes it wholly or leaves a fragment that capBody's partial-note guard (truncate.ts:171–180, matching on the shared head) drops — nothing stacks, and the bump is still right for the stated reason.

Site capability line — site/src/data/capabilities.ts:426–430 now carries the broadened scope and the opt-out.

Leftover polish (non-blocking)

src/lib/ai/prompt.ts:603–606: the author-notes section is still cut with a bare safeSlice(input.reviewNotes.trim(), 8000) while everything else this PR touches discloses its cuts — and manual runs now reach that path (reviews.ts:647 spreads ...notes into buildReviewPrompt). capBody(input.reviewNotes.trim(), 8000) would make it consistent; the "lives OUTSIDE budgetReviewExtras" rationale in the comment stays as-is.

Standing recorded decisions unchanged and not re-litigated: unscaled floors, the panel-vs-run notes fetch duplication, and useReviewerNotes's kind-less query key mirroring useExternalReviews.

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Round 4 dispositions — 3 nits + the leftover, all accepted and fixed in the next push

Distill length guard restored (nit). Both regressions you named were demonstrated against the unguarded path before fixing: an under-cap block with a real note had its indent moved to column 0 (and a near-cap block lost 2 further chars — real but small; the substantive wins are the other two), and a block merely quoting the note format got a fabricated reported=1234 on text nothing had clipped — shape-matching, exactly as you said. With the guard, under-cap blocks are byte-identical passthroughs and over-cap blocks are byte-identical to the round-3 path. The stale "no-op" sentence is deleted and the cumulative clause reworded to your shape.

newestAlone unreachability (nit) — comment appended with the arithmetic (every capped block ≤ 6,000 fits the 48,000 input cap's first iteration); the code stays as defense for constant convergence.

stripTruncationNote doc (nit) — reworded to name the two renderers as the indent sources and own-distill's bare re-cut input.

Leftover (author-notes bare slice) — fixed, batched with this push as the routing intends, and taken as a class. capBody(…, 8000) at the review-prompt site and at the description-generation sibling (~:300) — same idiom, same field, same silent-loss property; the note text is harmless-to-useful there since that section already asks the model to reflect decisions rather than paste them. The "lives OUTSIDE budgetReviewExtras" rationale stays. A >8K notes body now yields exactly one true-counted note (asserted: reported + kept === original); under-8K passes byte-identical. safeSlice's five other prompt.ts uses are deliberately untouched. Phrase-count invariants measured 10/7 before and after.

Verification: full scratch suite green (both sweeps zero-violation), tsc -b, scoped biome, pnpm build all green.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Round‑5 verification: all three nits from the last round and the leftover are applied, and I traced each fix plus its collateral (imports, doc sentences, the arithmetic the new comments assert). No blockers; one stale cross‑reference the notes fix introduced.

Resolved since last review

Distill length guard — own-distill.ts:55–59 restores if (b.length <= DISTILL_BLOCK_CAP) return b; before the strip/re-cap pair, so under-cap blocks (including ones legitimately carrying an indented note, and ones whose last line merely quotes the note format) pass through byte-identical, while over-cap blocks take the identical round-3 path. The stale "capBody is a no-op…" sentence is gone and replaced by an accurate rationale at :49–54, and the cumulative clause at :9–11 now reads "A block that exceeds this cap may already carry a note…". Re-checked the knock-on invariant: capBody still guarantees capped[i].length ≤ 6,000 (the priorOmitted > 0 && text.length ≤ cap path also lands within cap, since the reserved digit width uses text.length + priorOmitted ≥ omitted), so the fold arithmetic and DISTILL_INPUT_CAP walk are untouched.

newestAlone unreachability — own-distill.ts:68–73 now states the arithmetic (6,000 < 48,000 ⇒ keptCount ≥ 1) and that the branch is kept as defense; that claim holds given the invariant above.

stripTruncationNote doc — truncate.ts:48–51 now names formatOwnComments and formatExternalFindings as the indent sources and own-distill as the bare re-cut consumer. Verified against both renderers: own-context.ts:184–185 and external-context.ts:271 each call capBody with no indent and then .replace(/\n/g, "\n "), so the note really does arrive indented, and trimStart() in the parser handles it.

Leftover (author-notes bare slice) — prompt.ts:606 now uses capBody(input.reviewNotes.trim(), 8000), and the same fix was taken at the description-generation sibling prompt.ts:302. Both are byte-identical for notes under 8,000 chars. The capBody import is added and safeSlice is still live in that file (five uses at :1139, :1156, :1298, :1302, :1459), so no dead import.

Nits

nit — src/lib/ai/prompt.ts:603–604: the fix left the cross-reference "capped only by the 8000-char cap (same guard idiom as the plan-prompt issueBody slice below)" pointing at safeSlice(input.issueBody ?? "", 8000) at :1298, which is now a different idiom (undisclosed slice) — only the 8,000 size is shared. Reword to "capped only at 8,000 chars — the same size as the plan-prompt's issueBody slice below, which stays on safeSlice because it isn't a review prompt", or just drop the parenthetical.

Nothing further to raise — the notes wiring, the ReviewIgnoreOptions threading through the queued run, and the fair-share allocator all re-verified clean on this pass. (Copilot's capBody small-cap finding is addressed in the current code at truncate.ts:163.)

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Originally posted by: theBGuy

Round 5 — converged; merge-ready

The round closed every item on the board and raised one fresh optional nit. Disposition:

Stale cross-reference in the notes-cap comment (nit) — accepted, deferred on record. The parenthetical at prompt.ts:603–604 does now point at a different idiom (the plan-prompt issueBody slice stayed on safeSlice deliberately — it isn't a review prompt). It's a two-line comment reword with zero behavior, and a comment-only push would buy a full review round for it; per the convergence discipline it rides with the next PR that touches prompt.ts. Recorded here so it doesn't reopen as new.

Convergence state: round 4 returned only optional nits (all fixed in one push + the leftover batched per the routing clause); round 5 returned "nothing further to raise" plus this one comment nit. CI is green on the final head 052862f (build ×2, fragment, Cloudflare Pages), the single review thread (Copilot) is resolved with its disposition, the security audit was clean at round 1, and the three standing recorded decisions (unscaled floors · duplicate notes fetches · kind-less query keys) were respected without re-litigation across all five rounds.

Merge when ready — the merge is yours.

Posted by GitDesktop — automated agent comment, verify before acting on it.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-26

Ticket changed by: theBGuy

status: open --> closed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

feat(ai,pr-review): use author notes in manual reviews and fair-share...

Milestone

Searches

Help

#125 feat(ai,pr-review): use author notes in manual reviews and fair-share review-context budgets

Author notes in manual reviews

Opening-comment pin when own comments overflow

Documentation

Related

Discussion

Deploying gitdesktop with Cloudflare Pages

Context for reviewers

Disclosures

Verification

Related

Summary

Correctness

Docs

Readability / consistency (nits)

Recorded decisions acknowledged (not re-raised)

Related

Round 1 dispositions — 10 findings (2 reviewers), 9 accepted, 1 declined; all fixes land in the next push

Correctness

Docs

Nits

Resolved since last review

Round 2 dispositions — 3 findings, all accepted and fixed in the next push

Correctness

Docs

Resolved since last review

Round 3 dispositions — 3 findings, all accepted and fixed in the next push

Nits

Resolved since last review

Leftover polish (non-blocking)

Round 4 dispositions — 3 nits + the leftover, all accepted and fixed in the next push

Resolved since last review

Nits

Round 5 — converged; merge-ready

Latest commit:	`052862f`
Status:	✅ Deploy successful!
Preview URL:	https://41ff78af.gitdesktop.pages.dev
Branch Preview URL:	https://fix-own-comment-context-fair.gitdesktop.pages.dev

feat(ai,pr-review): use author notes in manual reviews and fair-share...

Milestone

Searches

Help

#125 feat(ai,pr-review): use author notes in manual reviews and fair-share review-context budgets

Author notes in manual reviews

Fair-share context budgeting

Opening-comment pin when own comments overflow

Documentation

Related

Discussion

Deploying gitdesktop with Cloudflare Pages

Context for reviewers

Disclosures

Verification

Related

Summary

Correctness

Docs

Readability / consistency (nits)

Recorded decisions acknowledged (not re-raised)

Related

Round 1 dispositions — 10 findings (2 reviewers), 9 accepted, 1 declined; all fixes land in the next push

Correctness

Docs

Nits

Resolved since last review

Round 2 dispositions — 3 findings, all accepted and fixed in the next push

Correctness

Docs

Resolved since last review

Round 3 dispositions — 3 findings, all accepted and fixed in the next push

Nits

Resolved since last review

Leftover polish (non-blocking)

Round 4 dispositions — 3 nits + the leftover, all accepted and fixed in the next push

Resolved since last review

Nits

Round 5 — converged; merge-ready