review(workflow): single-step review for daily drafts - #31
Merged
Conversation
Milestone 3 slice 4 (ADR 0008): any review_evaluation holder who is not a contributor decides a submitted draft — approve, request changes with a required comment, or return it. Self-review is refused for the owner, authors, the submitter, and ownership recipients; a coordinator who only moved ownership between others stays eligible, and a reviewer who requested changes may decide the resubmission. Decisions are permanent append-only rows paired with the review_decided event slice 3 reserved, audited, and announced to the owner; submission now nudges every reviewer but the submitter. Approval keeps the working copy frozen for finalization; a change request takes ADR 0008's second snapshot — anchoring exactly what the reviewer saw — and reopens the copy; a plain return reopens it without one. Migration 0009 holds the shape at the database: decisions land only on submitted drafts and never from contributors, and the 0008 freeze triggers are recreated onto the derived frozen state (submitted or approved), with one owner in the evaluation_record_frozen view. Web: a Review queue on home for reviewers, a decision panel on the workspace shown exactly when the caller may decide, verdicts and comments in the record, reopened drafts editable with the change request visible, and resubmission under the same revision contract. Closes the Milestone 3 exit: trainers document an invented session collaboratively without losing attribution, through review. Refs #30. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29b7ad097d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CI on the review PR caught programs::create_program failing with an
immediate SQLITE_BUSY ("database is locked", code 5) right after a
refused draft create, despite the 5-second busy timeout. The mechanism:
a refusal that returns while its BEGIN IMMEDIATE transaction is live
only queues the rollback on the connection's worker thread, so the
write lock briefly outlives the call - and SQLite does not consult the
busy timeout when a deferred transaction promotes an open read to a
write (verified: promotion fails in 0.000s while a fresh BEGIN
IMMEDIATE waits the full timeout). Any deferred writer meeting that
leftover lock fails instantly; a loaded runner loses the microsecond
race that a fast machine wins.
Every refusal path after storage::write_tx in evaluation_drafts,
draft_content, and draft_review now ends through storage::refuse, which
awaits the rollback so the lock is released before the refusal returns.
A regression test probes the property directly: after a refused create,
a connection with a zero busy timeout must take the write lock on the
first try. Converting the older deferred write paths themselves stays
tracked by #27.
Refs #30, refs #27.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a75ab2c5e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: the recreated update triggers checked only the old owning record, so a raw UPDATE could re-point a rating or narrative from an editable draft into a submitted or approved one — mutating a frozen working copy the backstop exists to protect. The modifier table had no update guard at all, leaving both re-pointing a modifier onto a frozen rating and swapping which modifier a frozen rating carries unchecked. The three update triggers (one new) now abort when either the old or the new owning record is frozen. A raw-database test moves a rating, a narrative, and a modifier from a same-version open draft into a frozen one, swaps a frozen row's modifier, and proves every case aborts with the frozen message and the frozen copy unchanged; against the old triggers the first move succeeded. Refs #30. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c167a1f057
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review, two backstop gaps in migration 0009. First, the comment
rule accepted whitespace padding on raw writes: length(' ') > 0, so a
change request could land without explaining itself. The CHECK now
trims ASCII whitespace (space, tab, LF, CR), mirroring the service's
Unicode trim. Second, the paired review_decided event lived only in the
service: a raw INSERT into review_decision committed a permanent
decision while the record still read as submitted, open to a second
decision on the same submission. The new
review_decision_advances_workflow trigger appends the event at the
database, making the decision and the state transition one atomic
write on every path; the service no longer appends it, and a second
raw decision now meets 'reviews decide submitted drafts'. A raw-write
test covers the blank comment, the auto-appended event, the double
decision, and that a service cycle still records exactly one event per
decision.
Refs #30.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3971b10b4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: the frozen view trusts the latest contributor event, but event inserts were unguarded — a raw append of contributed (or any non-review kind) after an approval made the approved copy read as editable again, and a bare review_decided event could thaw a submitted draft with no decision behind it. Two triggers close the class: while a draft is frozen no event lands except the review_decided its decision generates, and a review_decided event is accepted only as the pair of exactly one new decision row. With contributor events already append-only, the stream the derivation reads can now only move through the workflow's own transitions. Tests forge both directions raw — thawing a submitted draft, thawing and rerouting an approved one, and a decisionless review event — and prove every append aborts. Refs #30. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5712470e95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review, two follow-ups on the raw-decision surface the atomic transition opened. A raw changes_requested insert thawed the working copy without the change_request_return snapshot ADR 0008 requires — only the service took it. A new trigger refuses a change request until a return snapshot beyond those earlier change requests consumed exists, and the service now snapshots before inserting the decision row in the same transaction, keeping the one (Rust) owner of the snapshot format. And the comment normalization only trimmed space, tab, LF, and CR; a vertical-tab or form-feed comment still passed. The CHECK now trims the full Unicode White_Space set — exactly the characters the service's trim removes. Raw tests cover the unanchored change request, the exotic blank comment, and the anchored path reaching the comment rule. Refs #30. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8c566372d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: evaluation_session had no frozen-state guards, so a raw INSERT or DELETE could detach an approved record from the session it attests or attach another agreeing session — the permanent approval would then present against different coverage. Two triggers extend the freeze to coverage: while a record is submitted or approved, the sessions it documents can neither grow nor shrink (updates were never edits — 0008's keeps_identity). The raw-write test shrinks and grows a frozen record's coverage through an otherwise-valid same-enrollment session and proves both abort with coverage unchanged. Refs #30. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
This was referenced Aug 30, 2026
TusanHomichi
pushed a commit
that referenced
this pull request
Aug 30, 2026
…boundary The roadmap now states the current position AGENTS.md says it owns: Milestones 0-3 complete, Milestone 4 next with #32 as its design record. ADR 0010 makes the #31 review boundary durable — database backstops enforce properties of immutable record history (the self-review trigger is the model), while authorization and content semantics stay the domain services' typed, audited contracts; the rejected alternatives record why capability checks and content validation in triggers establish nothing against the writer they target. Content binding lands at Milestone 4 finalization hashes, where a verifier can actually check it. Refs #33. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primary Issue
Closes #30
Refs #22
Problem And Outcome
Milestone 3 slice 4 — the last slice: the review workflow over the draft substrate from #29. After merge, a submitted daily draft is decided by a qualified reviewer — approved, returned with change requests, or returned plain — completing the milestone exit: trainers document an invented session collaboratively without losing attribution, through review.
Changes
0009_review.sql:review_decision(reviewer, closed decision setapproved/changes_requested/returned, comment held non-blank for change requests by CHECK over the full Unicode White_Space set, decided instant) — append-only, accepted only while the record's latest contributor event is submitted-for-review, refused from any contributor, and requiring thechange_request_returnsnapshot ADR 0008 names before a change request lands; thereview_decision_advances_workflowtrigger appends the pairedreview_decidedevent at the database, so the decision and the state transition are one atomic write on every path; the 0008 freeze triggers are dropped and recreated (the 0006 precedent) onto the derived frozen state with one owner, theevaluation_record_frozenview: submitted and approved drafts are frozen, change-request and plain returns thaw the copyevaluation_sessioncoverage can neither grow nor shrink while frozen, and the contributor event stream itself is guarded — no event lands while a record is frozen except thereview_decidedits decision generates, and areview_decidedevent pairs with exactly one new decision rowstorage::refuse: every typed refusal afterBEGIN IMMEDIATEnow rolls back before returning, so the write lock never outlives the decision — a dropped transaction only queues its rollback, and SQLite refuses a deferred writer's read-to-write promotion immediately without consulting the busy timeout (the CI-onlydatabase is lockedfailure this PR surfaced; Write transactions: deferred snapshots can surface SQLITE_BUSY_SNAPSHOT as 500s under concurrent writes #27 keeps the deferred-path retrofits)draft_reviewmodule:decide— capability-gated (review_evaluation), state and eligibility rechecked inside the immediate write transaction; self-review refused for the current owner, actors of created/contributed/submitted events, and ownership recipients, while a coordinator who only moved ownership between others stays eligible and an earlier reviewer may decide the resubmission; a change request takes ADR 0008's second snapshot (change_request_return) anchoring exactly what the reviewer saw — inserted before the decision row whose triggers require it — then reopens the copy; every decision is audited (draft_review_decided) and notifies the owner (draft_approved/draft_changes_requested/draft_returned);queue— the currently submitted drafts with the caller's eligibilityDraftStatusgrowschanges_requested/returned/approved, still derived from the streams, never stored; save, transfer, and submit refuse frozen states with distinct typed refusals (draft_submitted,draft_approved) and resubmission repeats the submission contract (snapshot, freeze, viewed revision); submission now notifies everyreview_evaluationholder except the submitter; reviewers read records that have been submitted at least oncePOST /api/drafts/{id}/review,GET /api/reviews/queue; refusal mappingsself_review,not_submitted,comment_required,draft_approved; the workspace view carries the decisions with comments and aviewer_may_reviewflagtests/draft_review.rs— the eligibility matrix (no capability, self-review via a contributing coordinator, transfer-actor eligibility), the comment rule, the full submit → changes → revise → resubmit → approve cycle with thaw and freeze proven at the service and raw at the database, plain return without a snapshot, raw decisions advancing the workflow atomically (auto-paired event, refused double decision, unanchored change request, blank-comment forgeries), notices, audit counts, queue gating and drain, API round trip;tests/evaluation_drafts.rs— the write lock released on refusal, raw row moves into a frozen draft, frozen coverage;web/e2e/drafts.spec.tsextended to drive request-changes → revise → resubmit → approve in a real browserScope
Verification
Review And Merge Notes
review_decidedappend), theevaluation_record_frozenderivation with the guarded event stream and recreated freeze triggers, the eligibility rule indraft_review::decide, and the status extension's editability rulesreview_evaluationby default (the Administrator bundle is unchanged) — coordinators review🤖 Generated with Claude Code
https://claude.ai/code/session_01DNP3dkn9c84v2YoNwx7KTg
Generated by Claude Code