Skip to content

Write review feedback on a room candidate and repair from it (#173 PR 2) - #292

Merged
memtomem merged 1 commit into
mainfrom
feat/room-feedback
Nov 10, 2026
Merged

memtomem merged 1 commit into
mainfrom
feat/room-feedback

Conversation

@memtomem

Copy link
Copy Markdown
Owner

Summary

Second delivery of #173: review feedback on one Room candidate, and repairs that send it. You write change requests and questions on a candidate, optionally tied to a file it changed. room repair then sends that candidate's open requests to the member that produced it, and you record yourself whether the result met them.

syncmill room feedback add CAND --kind change --file calc.py --text "keep the old name"
syncmill room feedback add CAND --kind question --text "why a second cache?"
syncmill room feedback list ROOM [--candidate CAND]
syncmill room repair CAND --preview            # sends the open requests; --prompt is now optional
syncmill room repair CAND --confirm TOKEN
syncmill room feedback resolve FEEDBACK addressed|not-addressed|withdrawn [--on NEW]
syncmill room feedback carry FEEDBACK --to NEW

Refs #173. This is PR 2 of docs/room-targeted-repair-plan.md. Still to come: room diff --against and compare fields (PR 3), the TUI and --detach (PR 4).

How it holds together

  • Append-only rows, derived status. A new room_feedback table holds requests and resolutions. feedback_status derives open, sent, addressed, not_addressed, withdrawn or unknown on every read; nothing is stored. A damaged latest resolution reads unknown, never an older status.
  • sent needs a completed repair of the request's own candidate. A request named only by failed repairs stays open, so the next repair sends it again. After a completed repair the parent is no longer its member's newest completed candidate, so feedback carry writes the request again on the new candidate.
  • Only a human resolves. addressed and not-addressed name a later completed candidate of the same member, and its digest is recorded. Nothing reads an agent's output for a claim.
  • Confirmation binds the requests. The repair plan holds each request's record digest and status, and start_round's transaction re-derives them, so a resolve or withdraw between confirm and claim refuses before a provider starts.
  • Feedback text reaches only the repaired member's prompt. rounds.prompt stores the free text or the fixed line Repair candidate CAND, so apply shares no feedback to memory. The memory search uses the room's prompt, and room events carry ids only. Without requests, the repair task is byte-identical to PR 1's.
  • Writes change neither room state nor revision, so they cannot invalidate a confirmed selection. Writable in awaiting_decision and selected. prune_rooms deletes the rows before candidates.

The record set is checked as a whole

Status depends on how rows point at each other, and those links live in columns. Validating each row on its own leaked in three review rounds, each through another column combination. So before any request is read, feedback_inventory_problem checks the room's rows as a set:

  • canonical, unique ids
  • sequences exactly 1..N
  • each row is a request or a resolution
  • every resolution names an earlier request on its own candidate, in its column and its body alike
  • no other room's row names one of them

If any check fails, every request in the room reads unknown. Writing feedback, the repair batch and the claim are all refused. A damaged body on an intact set stays the concern of its own request, as the design says. A repair round whose provenance cannot be believed makes every request its raw record mentions unknown.

Known limit: deleting the newest row leaves no gap and is not detected, the same limit judgments have. It is noted in the code and in both user guides.

Decisions the design left open

Each was put to the design review:

  • a. Feedback is written only on completed candidates. A gate failure is fine.
  • b. carry targets a later completed candidate of the same member.
  • c. Multiple resolutions are allowed, and the latest wins. Repeating the current one is a no-op.
  • d. An anchor's path is checked against the candidate's files_changed when it is written. Readers check its shape only.
  • e. sent also requires the repair round's parent to be the request's candidate. A round naming a request on another candidate makes that request unknown.
  • f. Free text follows the requests, unlabeled.

Test plan

  • Full suite with suite-run: CLEAN (exit 0), and ruff check .
  • Each guard removed one at a time, and its test fails. Covered: the claim's feedback check, each record-set rule (two needed cases of their own, because another rule caught the first ones), resolution evidence, sent attribution, repair-round attribution, prune order, the stored prompt, the CLI's text suppression and NULL-id keying, carry's untrusted copy, and repository ownership
  • Memory: a sentinel in feedback text reaches the member's prompt, but not the search query or the task shared at apply
  • No real provider run. This changes what goes into the prompt, not how a provider is called; PR 1's smoke covered that path

Review

Codex, with each finding checked against the source:

  • Plan, 2 rounds: 3 Major and 1 Minor, then 0. Question anchors, resolution row binding, round-side checks of provenance ids, and an apply test.
  • Implementation, 4 rounds: 4, then 2, then 7, then 0.
    • Round 1: malformed value types, the text of an unknown entry, the ownership check on a withdrawal, preview counts.
    • Round 2: attribution by a damaged type column, C1 controls in an anchor path.
    • Round 3 was aimed only at rows edited directly in SQLite. Its findings led to the record-set check above; about 1 of the 7 came from an earlier fix.
    • Round 4 was given the diff alone, without the plan or a description of the changes: SHIP, 0 findings.

Found and left out of scope: a candidate row whose round_id points at another room's round is read unchecked (_list_candidates joins rounds without a room check). That is existing candidate-reading code, to be filed separately.

🤖 Generated with Claude Code

`room feedback add CAND --kind change|question [--file PATH]` records a
request on one completed candidate, optionally tied to a file it changed.
`room repair CAND` now sends that candidate's open requests by default
(`--feedback ID` narrows them), and `--prompt` becomes optional. `feedback
resolve` records, by a human only, whether a later candidate of the same member
met a request, and `feedback carry` writes it again on that later candidate.

Records are append-only rows in a new `room_feedback` table, and status is
derived on every read, never stored: open, sent, addressed, not_addressed,
withdrawn or unknown. A request reads `sent` only once a repair of its own
candidate completed, so a failed repair sends it again. The repair plan binds
each request's record digest and status, and the claim re-derives them, so a
resolve between confirm and claim refuses before a provider starts.

Feedback text reaches only the repaired member's prompt. `rounds.prompt` holds
the free text or a fixed line, so it stays out of what apply shares to memory,
and room events carry ids only.

Status hangs on how rows point at each other, and those links live in columns.
So the room's rows are checked as a set first: canonical unique ids, sequences
exactly 1..N, each resolution naming an earlier request on its own candidate in
column and body, no other room's row naming one. Any failure makes every
request in the room unknown and stops the writers, the repair batch and the
claim. A damaged body on an intact set stays the concern of its own request.
Deleting the newest row leaves no gap and is not seen, as with judgments.

Refs #173

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@memtomem
memtomem merged commit cb030c0 into main Nov 10, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant