diff --git a/contributing.md b/contributing.md index d044461..66f2e91 100644 --- a/contributing.md +++ b/contributing.md @@ -143,6 +143,56 @@ Send button. Never treat `{}` / no selection as "yes" for a public action. --- +## Review the code before every PR + +Review the branch after the code is written and **before** you build the review +commit, so the partner sees the cleaned-up version. Right-size the pass to the +change: a one-line or docs-only PR needs a careful reread, not a ritual audit; +a behavioral or structural change earns proportionally deeper review. Reading +and editing local source needs no approval and publishes nothing. + +Use two passes over the branch diff, in order. + +**Pass 1 — strip the slop.** Read your own diff as a hostile reviewer of +machine-written code and delete what a careful human would not have written: +comments restating what the line already says or breaking the file's existing +comment style, defensive `try`/`except` and existence checks on paths that are +already trusted, casts and broad types that only silence a complaint, nesting an +early return would flatten, and near-duplicates of a helper the codebase already +has. Behavior stays identical unless you are fixing a clear bug, and the edits +stay minimal and local. + +**Pass 2 — audit the structure the change itself motivates.** Ask whether the +new behavior sits at the layer that owns it and whether a simpler framing can +remove branches, flags, helpers, or layers. These are review signals, not +context-free blockers: + +- hand-written source crosses roughly 1000 lines because of the diff (generated + files, fixtures, data, and long-form prose do not count); +- new ad-hoc conditionals or special cases are bolted into a flow that did not + care about them before; +- feature-specific logic leaks into a shared or general-purpose path; +- an abstraction, wrapper, or layer of indirection is added without buying + clarity; +- optionality, `Any`, or loosely shaped dict payloads paper over an invariant + that should be explicit at the boundary; +- logic lands somewhere other than the layer that already owns the concept, or + duplicates a canonical helper; +- related updates can leave state half-applied, or independent work is + serialized for no reason. + +Act on findings that are motivated by this change and make the agreed behavior +clearer or safer, then re-read the diff. Keep unrelated refactors out of the PR. +When an obvious signal is deliberately left alone, record one sentence in the +private `plan.prior_work.summary` review evidence — never in the public +`body_draft` merely to narrate internal process. A larger finding becomes a +follow-up for the partner rather than silent scope expansion. + +If the instance has a richer code-quality or slop-removal skill installed, +apply it proportionally here too. + +--- + ## Prepare for review Nothing goes public here. For a PR, create a durable branch under `/data`, commit @@ -211,7 +261,8 @@ plan: {action: pr|issue|issue_comment|discussion_comment, # mirrors record.type stop #3). Compute the hash from the exact `.diff` bytes you store. Before you tell the partner it is ready, review the staged record yourself: -re-read the stored `.diff`, confirm the body draft is exactly what should be +re-read the stored `.diff`, confirm the proportional quality review above ran +against the committed branch, confirm the body draft is exactly what should be published, confirm no private data appears in the branch, commit message, branch name, body, or diff, and confirm the branch is back on `main` when the prep steps require it. diff --git a/mobius.json b/mobius.json index 43bcdf2..855e5f9 100644 --- a/mobius.json +++ b/mobius.json @@ -1,8 +1,8 @@ { "id": "contribute", "name": "Contribute", - "version": "0.5.9", - "description": "See what your agent has proposed upstream \u2014 pull requests, issues, and comments across the M\u00f6bius ecosystem, from prepared to merged.", + "version": "0.5.10", + "description": "See what your agent has proposed upstream — pull requests, issues, and comments across the Möbius ecosystem, from prepared to merged.", "author": "mobius-os", "license": "MIT", "homepage": "https://github.com/mobius-os/app-contribute", diff --git a/package.json b/package.json index 881b275..8fc7e7d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "app-contribute", - "version": "0.5.9", + "version": "0.5.10", "private": true, "scripts": { "test": "node --test test/*.test.mjs && python3 -m unittest discover -s test -p 'test_*.py'" diff --git a/review-followup.md b/review-followup.md index 950fc2c..f4e1252 100644 --- a/review-followup.md +++ b/review-followup.md @@ -82,16 +82,24 @@ you hold the live round. `` below is 4. **Do the work in the worktree.** Implement in-scope changes. A merge conflict requires a history rewrite that the current grant does not authorize: escalate it rather than rebasing or force-pushing. -5. **Run the project's tests** before pushing. If they still fail after two +5. **Run the proportional quality review** over what you just wrote — the two + passes in *Review the code before every PR* in + [contributing.md](contributing.md), applied **within this round's scope + only**. Your grant authorizes answering the review, not restructuring the + codebase: strip the slop and fix local structural problems motivated by your + new code. When the structural pass surfaces something genuinely larger, say + so in the `/reply` instead of doing it. A finding that needs a rewrite the + grant does not cover is an `/escalate`, never a quiet force-push. +6. **Run the project's tests** before pushing. If they still fail after two honest attempts, escalate — don't push red. -6. **Re-read the FULL diff.** Then write the new `head_sha` and `diff_sha256` +7. **Re-read the FULL diff.** Then write the new `head_sha` and `diff_sha256` onto the ledger record (a CAS storage write, same as preparing) so `/update` can bind to exactly what you reviewed. -7. **Push and reply.** `POST /update` with the new head; then `POST /reply` for +8. **Push and reply.** `POST /update` with the new head; then `POST /reply` for each thread you addressed, using its `in_reply_to` id when it is a review thread. Keep replies factual and scoped. Do not mark a draft ready or invent a review re-request; those are different GitHub actions. -8. **Complete.** `POST /complete` with `outcome` and a one-sentence summary. +9. **Complete.** `POST /complete` with `outcome` and a one-sentence summary. If a round has nothing to push (a plain question) you may `/reply` then `/complete` with `outcome: "replied"`. If you can do neither safely, `/escalate`.