GitDesktop / Tickets / #129 docs(comments): codify and apply the concise-constraint comment standard

Anonymous - 2026-07-28

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

Deploying gitdesktop with Cloudflare Pages

Latest commit: 03a0e90

Status: ✅ Deploy successful!

Preview URL: https://d6f20cf0.gitdesktop.pages.dev

Branch Preview URL: https://chore-comment-style-concise.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-28

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-28

Originally posted by: theBGuy

Context for reviewers — deliberate calls, each with its evidence. Later rounds can cite items by number.

What this is: a comments-only trim of 93 source files against a written standard (comments state what the code can't show; history and narration live in git), plus the standard itself in CONTRIBUTING.md, CLAUDE.md, and the gd-conventions skill. No runtime behavior, signatures, or control flow change.

The comments-only claim is mechanically proven, not asserted. Every TypeScript file: esbuild --minify-whitespace --minify-syntax output of HEAD vs this branch is byte-identical. Every Rust file: the diff contains only comment lines and adjacent blanks (grep-verified per batch). pnpm build green, cargo check (+--tests) green, all 872 Rust tests pass.

Every trim executed an itemized, adversarially-verified audit. Each DELETE/COMPRESS verdict was independently re-checked against the code by a second reviewing pass before application (~10% of verdicts were overturned there, and the corrected form is what shipped).

Stale comments were corrected, not deleted, where the underlying fact matters. Examples: automations/sync.ts's "non-draft" gate (drafts pass when reviewDraftPrs is on — the comment now says so), usePrCapabilities.ts's provider lists (checked against forge/model.rs flags), bitbucket.rs pagination claims (ten next-following readers, verified). If a rewritten fact looks surprising, it was code-verified before it was written.

Empirically-learned API/platform facts were kept even where they sound anecdotal — tier clamps, retired endpoints, pagelen caps, Windows registry-PATH behavior, .exe-over-.cmd shim rules. What was stripped is the provenance framing around them (dates, PR numbers), which git history owns.

docs/*.md pointers in comments are kept deliberately. /docs/ is gitignored by design (local design docs); 14 comment pointers into it remain. They dangle in a fresh clone — known, recorded, and the maintainer's call to strip later if wanted.

No changelog fragment, deliberately. Comments-only + contributor docs = no user-facing surface; the no-changelog label is the fragment gate's designed escape hatch and is applied.

own-distill.ts keeps measured calibration figures (payload sizes, a timed CLI run) with their source cited. Those are measurements the constants were derived from, not change-narration, and the standard carries an explicit carve-out for exactly this class.

Round-0 CI on the draft was red; both causes are handled in the next push. (a) clippy doc_lazy_continuation ×4 — one doc block in forge/jira.rs where a line beginning + the parsed as a Markdown bullet; fixed by + → plus, and the gd-conventions section now warns about this exact clippy surface. (b) the fragment gate — see item 7. Local cargo clippy --all-targets -- -D warnings is clean.

A pre-open shadow review ran over this diff; its fixes ride the same push. Corrected: a stale "GitLab-only" approval doc in forge/model.rs (Bitbucket shares the bodyless toggle — the TS twin was already corrected by the trim) plus its "the one write GitHub leaves false" counting word (it's three); "true for both" on mr_merge (all three providers merge); a doc pointer whose document name the trim had orphaned (model.rs § reference); a caller-validates invariant restored on github_sort_args; one "all have their analogue" → "mostly" in a test comment.

Disclosure — five pre-existing code issues found during the trim, deliberately not fixed here (comments-only invariant; all tracked in the maintainer's backlog): github/pr.rs commit_comment_create resolves via the lens while edit/delete resolve via origin (fork-lens mismatch); JiraIssueView.tsx JiraEstimateInput's invalid flag never resets on a server-value change; ops.rs mojibake (â€") ×8 including one user-facing toast string in rewrite_commits; repo-data-migration.ts mergeIds lets two id-less records in one legacy array both survive; git/diff.rs's pathspec-glob doc contradicts compare.rs's corrected twin.

Disclosure — useLinkedIssueChips.ts lost its two "Reasoning (no test runner)" example tables. The invariant sentence (exactly ONE of refs/jiraRefs) survives verbatim; the per-case rows were compressed under the worked-example rule. This repo has no frontend test runner, so if those rows are wanted back as a test substitute, say so — cheap restore.

Disclosure — the ~500 remaining low-comment-density files are trim-on-touch by design (CONTRIBUTING's new section records the standard); three repo-wide greps (change-narration markers, drifted count claims, dangling doc pointers) already swept them. The densest untouched file, mcp_server/write_local.rs, is deliberately in that bucket.

Disclosure — some files' working-copy line endings normalized during editing. Content-neutral (numstat shows small balanced counts, not whole-file flips); git/CI normalize on checkout.

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-28

Originally posted by: theBGuy

🤖 GitDesktop AI security audit · opus · automated

No genuine security issues in these changes: the diff is comment/doc text only — every hunk I can see (Rust ////// blocks, the CONTRIBUTING/CLAUDE/skill sections) leaves code, argv construction, and the credential/sandbox guards byte-identical, and I verified the two claims a trimmed comment now rests on (bitbucket_credential_entries' -c key is always url.-prefixed so it can't reach an arbitrary git config key; credential_config_for_remote routes to that arm only for bitbucket.org).

Coverage note: the diff was truncated after forge/gitlab.rs, so the remaining ~50 files (TS src/**, git/*.rs, mcp_server/*, pty.rs, path_launcher.rs, …) were not reviewable here — this verdict covers only what was shown.

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-28

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus

Summary

A comments-only trim of 93 files against a newly written standard (CONTRIBUTING.md, CLAUDE.md, gd-conventions skill). I spot-verified the rewritten claims against the code at head — usePrCapabilities.ts's entire provider matrix against Implemented::for_provider, sync.ts's draft gate against reviewDraftPrs: false (settings/api.ts:324), http.rs's new "detail-over-message" test note against BbErrorBody::best_message, the 35% pin / 6,000 floor / 48K cap / "≥8 blocks" arithmetic across truncate.ts ↔ own-context.ts ↔ own-distill.ts, and LocalPrMergeOutcome.base_tip's new "start on the initial call, merged commit on finish" against ops.rs:2104/2272 — and they hold. Nothing blocking; four items below, two of them stale-claim siblings the sweep missed.

Accuracy of the rewritten comments

should-fix — src-tauri/src/forge/model.rs:819-821, in bitbucket_implements_pr_and_ci_writes: "MR merge is a shared control (both providers); approve/unapprove is the one GitLab-only write — GitHub approves via the review flow, not this toggle." Both halves are stale and contradicted inside this same file: mr_approve's field doc, corrected by this PR at :151-153, now reads "GitLab and Bitbucket share it"; for_provider(Bitbucket) sets mr_approve: true at :535; and the assertion 10 lines below at :830 is assert!(bb.mr_approve). Merge is likewise all three now (:155-156 "A shared control on all three providers", mr_merge: true at :532). Your notes record correcting the "GitLab-only" approval doc in model.rs — this is its sibling in the test comments, left behind. Fix: "MR merge is shared by all three providers; the bodyless approve/unapprove toggle is GitLab + Bitbucket — GitHub approves through the review flow, not this toggle." Nothing else needs to move; the two assertions already match.

nit — src-tauri/src/agent.rs:78-79, AgentKind: "all four run reviews + sessions, host or container" reads as if a review can run containerized. agent_review (:1739) has no isolation/container parameter; container isolation exists only on agent_session (:1937). Suggest "all four run reviews (host) and sessions (host or container)".

The standard itself

should-fix — CONTRIBUTING.md (Code comments) and CLAUDE.md (house-rules bullet) state the rule without the exception the skill grants: .claude/skills/gd-conventions/SKILL.md carries "Carve-out: measured figures a later reader would otherwise have to re-measure (payload sizes, timed runs) may stay and cite their source" plus the "~6 for genuinely multi-constraint blocks" allowance, while CONTRIBUTING says "three lines is plenty" and CLAUDE.md says flatly "Never change-history or PR references". This PR itself relies on the carve-out — src/lib/ai/own-distill.ts:79 and :102 deliberately keep "the real payload from PR [#125] … 135s on claude -p --model opus" (your item 8) — and CLAUDE.md is the auto-loaded copy, so an agent applying it verbatim on the next touch of that file deletes exactly what the skill says to keep. Fix: add the same one-clause carve-out (and the multi-constraint-block allowance) to the CONTRIBUTING "Code comments" paragraph and the CLAUDE.md bullet, so all three copies say the same thing.

Consistency of the sweep

nit — the phase/wave/round/spec provenance framing the standard forbids survives in files this PR rewrote (grepped tree-wide; untouched files excluded per your item 13): forge/model.rs:825,827,837,839 ("Phase 3", "Phase 4 PR writes", "wave 2/3", "wave 4's"), forge/bitbucket.rs:6727 ("── Wave 2/3 (publish + repo management) unit tests ──"), forge/mod.rs:3573 ("gh / Phase 1 decide"), git/remote.rs:1135 ("the round-3 exact-match guard, end to end" — whose sibling "Round-3 regression guard" was rewritten in this diff), and mcp_server/generate.rs:1233 ("mirrors the spec's best-effort contract", pointing at a document not in the repo). Each drops to the same claim minus the provenance clause.

Other reviewers

Copilot's two findings (oplog.rs:18, local_prs.rs:16) are false. Both blocks have balanced code spans — oplog.rs:17 opens `~/Library/Application and :18 closes it, then opens and closes two more; local_prs.rs:15-16 is the identical wrapped span — and rustdoc strips the //! prefix before markdown parsing, so no stray backtick or unclosed span renders.

Recorded decisions (acknowledged, not re-raised)

No changelog fragment / README entry (item 7): comments plus contributor docs, nothing user-facing — CHANGELOG.md, README.md and changelog.d/ are correctly untouched. The kept docs/*.md comment pointers (item 6) check out as recorded — .gitignore:30 does ignore /docs/, so they dangle for anyone else's clone; still your call. useLinkedIssueChips.ts's dropped tables (item 12) are fine as shipped: the ONE-of-refs/jiraRefs invariant plus a two-line worked example per kind survives at :46-52.

Coverage note: the prompt's diff was truncated, so I read the omitted files directly at head. The repeated-pattern sweeps above (provenance framing; "both providers"/"GitLab-only" phrasing) were grep-driven across the whole tree and are complete; the remaining prose was sampled, not read line-by-line.

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

Related

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

Anonymous - 2026-07-28

Originally posted by: theBGuy

Dispositions for the AI review's items — all verified against code before acting; fixes land in the next push (the closing batch).

model.rs test comment (should-fix): Fixed — your wording applied after re-verifying it against the assertions (gh.mr_merge && !gh.mr_approve at the site, assert!(bb.mr_approve) ten lines down). The find also exposed that this staleness class crosses the language boundary, so the same batch corrects the TS side: types.ts mrMerge ("both GitHub and GitLab" → all three) and mrAutoMerge's now-broken "like mrApprove" analogy, api.ts's forgePrApprovals doc, three sites in RemotePrView.tsx (fetch-gate comment, optimistic-flip key comment, approval-display label), and forge_pr_approvals' own "GitLab-only" doc in mod.rs — its body has had a Bitbucket arm all along.

agent.rs reviews-vs-sessions (nit): Fixed — "all four run reviews (host) and sessions (host or container)", after confirming agent_review takes no isolation parameter and the frontend invoke passes none.

Standard carve-out (should-fix): Fixed — the measured-figures carve-out (with the "a later reader would have to re-measure" qualifier) and the ~6-line multi-constraint allowance now appear in all three copies: CONTRIBUTING.md, CLAUDE.md, and the gd-conventions skill. You were right that the auto-loaded copy was the one that would have deleted exactly what the skill protects.

Provenance survivors (nit): Fixed at 11 sites — your list (model.rs ×4, bitbucket.rs banner re-padded to its siblings' 83-char width, mod.rs, remote.rs, generate.rs) plus two siblings the class grep found in jira.rs tests (phase-5/6 permission-key comments, the "B5" edited-cue reference). Two grep hits deliberately kept: readme.ts's "Phase 0–4" are the distiller's own pipeline stages (algorithm vocabulary, not project history), and the gd-conventions "spec's Docs-sync field" line is delegated-package vocabulary. Files outside this PR (insights.rs, write_local.rs, RemoteIssueView.tsx, PrCommitDetail.tsx, branch-rules/types.ts) stay per recorded item 13 — trim-on-touch, already itemized in the maintainer's audit ledger.

Copilot verdict concurrence — we reached the same balanced-spans refutation independently; both threads are replied and resolved with the defensive one-span-per-path rewrite riding this batch anyway.

Also in the batch, from a post-fix shadow pass over the fixes themselves: my first rewrite of the mod.rs host-routing test comment traded vague for wrong (the gh default does not handle self-managed GitLab — detect_non_github resolves it via glab's known hosts; corrected), and generate.rs's rewritten labels comment briefly restated itself (tautological parenthetical dropped).

Gate on the closing batch: cargo clippy --all-targets -- -D warnings clean, pnpm build clean, biome clean on touched files (api.ts's whole-file CRLF complaint is the pre-existing checkout condition, line 1 is an untouched import), and esbuild minify equivalence re-proves comments-only for every touched TS file.

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-28

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus

Summary

Re-review of the fix batch on a comments-only trim: all four previous findings are corrected in the current tree, and I verified each against the code it describes. The one new thing: the fix commit's own stated sweep ("approve/unapprove and request-changes are GitLab and Bitbucket controls, not GitLab-only") corrected five sites but missed three siblings, two of them in the same file and function it edited. Nothing blocking.

Accuracy of the rewritten comments

should-fix — the "GitLab-only approval" sweep left three survivors, each contradicted by code in view:

src-tauri/src/forge/mod.rs:1610-1611, forge_pr_unapprove: "Revoke the viewer's approval of a merge request, behind the abstraction. GitLab-only." — its Bitbucket arm is five lines below at :1616 (bitbucket::unapprove_pr, which DELETEs …/pullrequests/{n}/approve). 6d5a5d9 rewrote forge_pr_approvals' doc 38 lines above this one and stopped. Fix: "GitLab and Bitbucket; GitHub approvals go through the review flow, so its arm errors."

src-tauri/src/github/pr.rs:1985-1989 + :2006, ApprovalState: "Provider-neutral, but only GitLab produces it today" and "the GitLab-only approve/unapprove control" / "the GitLab-only Request-changes control's pressed state". bitbucket::pr_approvals (bitbucket.rs:3161) returns this exact struct, and build_approval_state (:3109-3112) is what fills viewer_requested_changes from the participant state. Fix: "produced by GitLab and Bitbucket" and drop "GitLab-only" from both control references; while there, viewer_requested_changes' note that "the direct undo mutation is Premium-only" is GitLab-specific — attribute it (Bitbucket's revoke works on every plan, as usePrCapabilities.ts:30-37 already records).

src/features/pulls/RemotePrView.tsx:520: "GitLab approve/unapprove — one toggle keyed on whether the viewer approved." — this is the header of toggleApproval, whose inner comment at :528-529 the same batch rewrote; the toggle gates on canApprove (= mrApprove, true for GitLab and Bitbucket) and dispatches unapprovePr → forge_pr_unapprove's Bitbucket arm. Fix: "GitLab + Bitbucket approve/unapprove — one toggle…", and attribute the user_can_approve sentence that follows to GitLab (it's a GitLab Free-tier quirk, not a shared one).

nit — src/features/pulls/RemotePrView.tsx:837-838: the display is now labelled "(GitLab + Bitbucket)" but the second clause still reads "or a Premium project requires N approvals" unattributed; build_approval_state (bitbucket.rs:3081-3082) always returns approvals_required = 0, so that branch is GitLab-only — say "or a GitLab Premium project requires N approvals".

nit — src-tauri/src/forge/model.rs:151-153, mr_approve: "one of the three writes GitHub leaves false (see all)" — ten flags are false for GitHub in all(); the three you mean are the ones whose GitHub analogue lives elsewhere. Say "one of the three writes whose GitHub analogue lives elsewhere (see all)".

nit — src-tauri/src/forge/mod.rs:2969-2971, forge_repo_set_visibility: "both providers take public/private/internal" — the match has three arms and Bitbucket explicitly rejects "internal" (:2979-2981). Same two-provider-era wording class as the above; say "all three take public/private, with internal GitHub/GitLab-only".

The standard itself

nit — CLAUDE.md:61 still states "Never change-history or PR references" while the new carve-out at :63-64 says measured figures "may stay and cite their source", and the only in-repo instances of that carve-out cite a PR number (src/lib/ai/own-distill.ts:79, :102 — "PR [#125]"). One clause closes it for good: "…may stay and cite their source (a PR or run reference is fine there)". CONTRIBUTING.md:100-103 and .claude/skills/gd-conventions/SKILL.md carry the same phrasing and would want the same clause.

Readability

nit — two lines the edits left mis-wrapped: CLAUDE.md:64-65 ("…re-measure may stay and cite their source.) Trim / any comment you touch to this standard.") breaks mid-sentence at ~half width, and RemotePrView.tsx:837 now runs 92 chars against its neighbours' ~80. Rewrap both.

Resolved since last review

forge/model.rs:819-821 test comment (should-fix) — corrected; re-verified against the assertions it describes (assert!(gh.mr_merge && !gh.mr_approve) at :822, assert!(bb.mr_approve) at :831, mr_merge: true for all three profiles).

agent.rs:78-79 AgentKind (nit) — now "all four run reviews (host) and sessions (host or container)"; agent_review (:1739) takes no isolation parameter, agent_session (:1935-1966) does, and agent_sandbox::agent_npm_package covers all four CLIs.

Standard carve-out (should-fix) — CONTRIBUTING.md:97-106 and CLAUDE.md:58-65 now carry both the ~6-line multi-constraint allowance and the measured-figures carve-out, matching the skill (subject to the wording nit above).

Provenance survivors (nit) — fixed at every site listed: model.rs:826/828/838/840, bitbucket.rs:6727 (banner re-padded, matches its siblings at :6584/:6634), mod.rs:3573-3574 (the rewritten routing claim checks out — provider_for_host returns None for gitlab.acme.com and detect_non_github resolves it via glab's known hosts), remote.rs:1135, generate.rs:1233, plus the two jira.rs test siblings and the B5 reference. A tree-wide re-grep leaves only files this PR doesn't touch (insights.rs, branch-rules/types.ts) and algorithm vocabulary (readme.ts's Phase 0–4, the "round 1/round 2" two-pass allocation in own-context.ts / own-distill.ts / external-context.ts) — correctly kept.

Copilot's two code-span reports (oplog.rs:18, local_prs.rs:16) — the current text carries the one-span-per-path rewrite ($XDG_DATA_HOME/<id>/ (or ~/.local/share/<id>/)), which also matches dirs::data_dir()'s actual Linux resolution; the reported render breakage was never real.

Recorded decision, unchanged: no changelog.d/ fragment, CHANGELOG.md or README.md entry — comments plus contributor docs, nothing user-facing, so those surfaces are correctly untouched.

Coverage note: this round I re-verified the delta hunks in full plus the sites they touch, and re-ran the stale-claim and provenance sweeps tree-wide; the rest of the PR was covered by the previous pass and is unchanged since.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready

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

Related

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

Anonymous - 2026-07-28

Originally posted by: theBGuy

Round-2 dispositions — all items fixed in the final batch, each verified against code before adopting the suggested wording.

The three "GitLab-only approval" survivors (should-fix): Fixed. forge_pr_unapprove's doc (its Bitbucket arm is five lines below it), ApprovalState's "only GitLab produces it today" + both "GitLab-only" control references in github/pr.rs (with the Premium-undo note now attributed to GitLab), and RemotePrView's toggleApproval header (with user_can_approve attributed to GitLab). You called the root cause correctly: round 1 fixed the enumerated sites, not the phrase-class. This time the grep ran over every PR-touched file — the remaining "GitLab-only" hits are all genuinely GitLab-only features (auto-merge, time tracking, related issues, confidential/due dates).

Premium attribution in the approval display (nit): Fixed — "a GitLab Premium project requires N approvals" (verified: Bitbucket's build_approval_state always returns approvals_required = 0), and the line re-wrapped to its neighbours' width.

"One of the three writes" (nit): Fixed — now "one of the three writes whose GitHub analogue lives elsewhere (see all)", matching all()'s own enumeration.

forge_repo_set_visibility "both providers" (nit): Fixed — "All three take public/private; internal is GitHub/GitLab-only — Bitbucket rejects it" (verified: the Bitbucket arm errors on internal, the GitHub arm passes through to gh).

Carve-out vs "Never PR references" (nit): Fixed in all three copies — "(a PR or run reference is fine there)" added to CONTRIBUTING, CLAUDE.md, and the gd-conventions skill, so the own-distill.ts citations are square with every copy of the rule.

Wrap defects (nit): Fixed — the CLAUDE.md bullet tail re-wrapped (no more mid-sentence half-width break) and the RemotePrView display comment brought back under width.

Gate on the batch: cargo clippy --all-targets -- -D warnings clean, pnpm build clean, biome clean on the touched file, esbuild minify equivalence re-proves comments-only, and the stale-phrase class grep over every PR file returns zero survivors.

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-28

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus

Re-review of the round-2 fix batch (six files, all documentation/comment text). Every previous finding is corrected in the current tree and I verified each against the code it describes; the only things left are re-wrap artifacts the fixes themselves introduced. Nothing blocking.

Readability

nit — the fix batch edited three paragraphs without re-flowing them, leaving a mid-sentence half-width break in each: src/features/pulls/RemotePrView.tsx:522 (// approving works), so — ~25 chars against its neighbours' ~85), CONTRIBUTING.md:103 (run reference is fine there). When you / touch a file,), and src-tauri/src/github/pr.rs:1988 (/// not a bodyless toggle, so its forge arm errors and the — ~58 chars against ~80). Re-flow each of the three paragraphs to its file's prevailing width; no wording change is needed.

nit — src-tauri/src/forge/mod.rs:1610, forge_pr_unapprove: the headline still reads "Revoke the viewer's approval of a merge request" while the line just rewritten below it covers Bitbucket too and the sibling forge_pr_approve (:1590) says "a merge/pull request". Say "merge/pull request" here as well.

Resolved since last review

forge/mod.rs:1610-1612 forge_pr_unapprove (should-fix) — now "GitLab and Bitbucket; GitHub approvals go through the review flow, so its arm errors"; verified against the body: GitLab arm :1616, Bitbucket arm :1617, GitHub arm returns InvalidArgument at :1618-1620.

github/pr.rs:1985-1989 + :2005-2008 ApprovalState (should-fix) — "produced by GitLab and Bitbucket" and the un-attributed "GitLab-only" control references are gone; confirmed bitbucket::pr_approvals (bitbucket.rs:3161) returns this struct and build_approval_state (:3109-3112) fills viewer_requested_changes from the participant state. The Premium-undo clause is now correctly attributed to GitLab.

RemotePrView.tsx:520 toggleApproval header (should-fix) — now "GitLab + Bitbucket"; verified the toggle renders only behind canApprove (:1830) and toggleApproval is its sole call site (:1842), so the GitHub review-flow path (SubmitReviewDialog, :2362) never reaches it. user_can_approve is now attributed to GitLab.

RemotePrView.tsx:838-840 Premium attribution (nit) — now "a GitLab Premium project"; matches bitbucket.rs:3116 (approvals_required: 0, documented at :3081-3082).

model.rs:151-154 mr_approve (nit) — "one of the three writes whose GitHub analogue lives elsewhere (see all)" now reconciles with all()'s own doc at :296-299, which names exactly mr_approve / mr_request_changes / mr_draft_toggle (all three false in that profile).

forge/mod.rs:2970-2971 forge_repo_set_visibility (nit) — "All three take public/private; internal is GitHub/GitLab-only" matches the Bitbucket arm's explicit internal rejection at :2979-2981.

Carve-out vs "never PR references" (nit) — the "(a PR or run reference is fine there)" clause is present in all three copies (CONTRIBUTING.md:102-103, CLAUDE.md:64, .claude/skills/gd-conventions/SKILL.md:154-155), squaring the rule with own-distill.ts:79/:102's PR [#125] citations.

CLAUDE.md:62-65 wrap (nit) — re-wrapped cleanly. (The sibling wrap in CONTRIBUTING.md was not; see the nit above.)

Coverage note: I re-verified every delta hunk in full plus the code each rewritten claim describes, and re-ran the stale-claim sweeps over the PR-touched files — the remaining GitLab-only hits are genuine (auto-merge, time tracking, issue links, confidential/due date, manual job play), model.rs:163/:227's "both" correctly means GitHub+GitLab (Bitbucket has neither labels nor issues), and the provenance grep leaves only readme.ts's Phase 0–4 algorithm vocabulary, own-distill.ts's carve-out citations, and files this PR doesn't touch. The remainder of the PR is unchanged since the previous pass, which covered it.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready

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

Related

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

Anonymous - 2026-07-28

Originally posted by: theBGuy

Round-3 dispositions — all four nits fixed in one final polish batch (4 files, 9 lines: the three paragraphs re-flowed to their files' prevailing width, and forge_pr_unapprove's headline now says "merge/pull request" matching its forge_pr_approve sibling). Verified: clippy clean, esbuild minify equivalence still proves comments-only on the touched TS file.

Convergence: two consecutive rounds have returned no blocking issues and this round's only findings were re-wrap artifacts of the previous round's own fixes — the review is polishing, not finding. With CI green on the final head and both Copilot threads resolved, this PR is merge-ready once the polish batch lands.

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-28

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Round-4 re-review of the polish batch: four comment/doc-text edits (three paragraph re-flows plus one doc headline reword) across CONTRIBUTING.md, src-tauri/src/forge/mod.rs, src-tauri/src/github/pr.rs, and src/features/pulls/RemotePrView.tsx. All four previous findings are corrected, the fixes introduce no collateral (no wording changed except the intended merge/pull request headline, no doc block detached from its symbol), and nothing blocking remains.

Coverage: the diff shown here omits 73 files, so this round's own scan covers only what is displayed; the delta since the last review is shown in full and I verified each of its hunks against the code it describes.

Resolved since last review

CONTRIBUTING.md:100-104 (nit) — the mid-sentence half-width break is gone; the paragraph now flows to the file's prevailing width and the carve-out clause reads intact.

src-tauri/src/github/pr.rs:1985-1989 ApprovalState (nit) — re-flowed; the "so its forge arm errors and the approve/unapprove control gates on implemented.mrApprove (false for GitHub)" sentence now sits on even-width lines with no wording drift.

src/features/pulls/RemotePrView.tsx:520-525 toggleApproval header (nit) — re-flowed to ~80 chars; the user_can_approve-is-unreliable-on-Free clause and the optimistic-flip rationale are unchanged.

src-tauri/src/forge/mod.rs:1610 forge_pr_unapprove (nit) — headline now reads "Revoke the viewer's approval of a merge/pull request", matching forge_pr_approve at :1590 and the two-provider body line beneath it.

Leftover polish (non-blocking)

src-tauri/src/agent.rs:6 (module doc, unchanged since the last review) — "Reviews are read-only: Tier 1 exposes no tools at all" is absolute, but the same file's claude_review_args sets tools = String::new() only for the base set and then appends the mcp__<server> patterns, which its own comment states ("Diff-only exposes no base tools; MCP patterns are still appended so a loaded self-server is callable"). Suggest: "Reviews are read-only: Tier 1 exposes no base tools — only an attached, read-only self-MCP's."

Nothing further to raise. The change is comments plus contributor docs with no user-facing surface, so README / CHANGELOG / changelog.d/ need no entry here; the no-fragment call is recorded in the author's note (item 7) with the no-changelog label.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready

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-28

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:

docs(comments): codify and apply the concise-constraint comment standard

Milestone

Searches

Help

#129 docs(comments): codify and apply the concise-constraint comment standard

Documentation of the standard

Rust backend trim (`src-tauri/`)

Frontend trim (`src/`)

Related

Discussion

Deploying gitdesktop with Cloudflare Pages

Summary

Accuracy of the rewritten comments

The standard itself

Consistency of the sweep

Other reviewers

Recorded decisions (acknowledged, not re-raised)

Related

Summary

Accuracy of the rewritten comments

The standard itself

Readability

Resolved since last review

Related

Readability

Resolved since last review

Related

Resolved since last review

Leftover polish (non-blocking)

Latest commit:	`03a0e90`
Status:	✅ Deploy successful!
Preview URL:	https://d6f20cf0.gitdesktop.pages.dev
Branch Preview URL:	https://chore-comment-style-concise.gitdesktop.pages.dev

docs(comments): codify and apply the concise-constraint comment standard

Milestone

Searches

Help

#129 docs(comments): codify and apply the concise-constraint comment standard

Documentation of the standard

Rust backend trim (src-tauri/)

Frontend trim (src/)

Related

Discussion

Deploying gitdesktop with Cloudflare Pages

Summary

Accuracy of the rewritten comments

The standard itself

Consistency of the sweep

Other reviewers

Recorded decisions (acknowledged, not re-raised)

Related

Summary

Accuracy of the rewritten comments

The standard itself

Readability

Resolved since last review

Related

Readability

Resolved since last review

Related

Resolved since last review

Leftover polish (non-blocking)

Rust backend trim (`src-tauri/`)

Frontend trim (`src/`)