Skip to content

feat(review): ask where else a fact is written and what holds the copies together - #158

Merged
mgratzer merged 1 commit into
mainfrom
claude/review-restated-facts-156-d10277
Nov 10, 2026
Merged

mgratzer merged 1 commit into
mainfrom
claude/review-restated-facts-156-d10277

Conversation

@mgratzer

@mgratzer mgratzer commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

The default review now asks where else a constant, bound, vocabulary or rule is written and what holds the copies together, so a fact restated in another tier or language is found at the diff, not after it lands.

Notes

  • Judgment calls (HITL Issue, run unattended): the question uses the Issue's proposed wording verbatim, as a fifth Design question. The cut is the Issue's first candidate: item 5 drops "hardcoded values that should be constants", which the new question covers from the other side (a value worth a constant is one written in more than one place). Net words still grow by about 30 across the two modules; folding it into the sibling question would have made that question carry two searches.
  • The rubric's P1 examples gain "a constant, bound or rule restated elsewhere with nothing holding the copies together", beside the copied module.

Exercised

  • forge-new:forge-reflect (this branch) on a sandbox branch adding GET /notes/:id/preview plus a client that restates the 80-char length (with a "keep in sync" comment) and the id regex (without one) → P1 on the restated length, naming the id regex at src/server.js:17 and the third copy in docs/api.md; plus 4 P2s
  • forge-old:forge-reflect (base 24f900e), same branch → P1 on the restated length only, no mention of the id regex; same 4 P2s, plus a tracker-spec P1 (the new run raised it as an open question) and one on the fixture's commit title
  • Not exercised: a full forge-ship --guard run; the sandbox branch stayed local, no sandbox PR opened

Plan

Plan as first made (no approver: run unattended, started by another session with --guard)

Deviations (as of c0b492e): none

Closes #156

Risk

Verdict: needs review, judged on c0b492e

  • Peer review: none configured
  • Size: 55 counted lines (0 excluded) → ok
  • Always-review paths: none
  • Tests: ok
  • Exercise: ok
  • Plan: ok
  • Design: ok
  • HITL: feat(review): ask where else a fact is written and what holds the copies together #156 shipped without an approved Plan, judgment calls listed
  • Trust: ok
  • Deployment: none
  • What the checks would miss: The cut the Issue left to the maintainer. Item 5 loses "hardcoded values that should be constants", which also covered a single magic number written only once, and the new question does not cover that case. The new P1 example could also raise how often reviews block on hand-kept mirrors that are intentional.

Where to look (needs review only)

  • skills/_shared/review-dimensions.md:9: The Issue left two cuts to choose from. Is adding a fifth Design question (about 30 more words) better than folding this into the sibling question, as the PR's Judgment calls Notes bullet argues?
  • skills/_shared/review-dimensions.md:16: With "hardcoded values that should be constants" gone from item 5, will a review still flag an unexplained magic number that appears only once?
  • skills/_shared/review-rubric.md:8: Should a restated fact with nothing holding the copies together be a P1 ("should fix before merge"), or does that over-block mirrors that are only kept in sync by a comment?

@mgratzer
mgratzer merged commit cc9fad5 into main Nov 10, 2026
2 checks passed
@mgratzer
mgratzer deleted the claude/review-restated-facts-156-d10277 branch November 10, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): ask where else a fact is written and what holds the copies together

1 participant