Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions mobius.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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'"
Expand Down
16 changes: 12 additions & 4 deletions review-followup.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,24 @@ you hold the live round. `<base>` 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`.
Expand Down
Loading