docs(references): add PRD-0017 and ADR-0018 for interactive review sessions#151
Conversation
…ssions PRD-0017 specifies the agent<->operator feedback loop on a wiki page, ported from the lavish-axi local-artifact review pattern but keyed to (wiki_id, title) targets instead of file paths. ADR-0018 records the structural decisions: pure session core in sp42-platform, in-memory store with Notify wake-ups in sp42-server, /dev/review/* routes gated by the bridge session, and Parsoid block/cite anchors in place of CSS selectors. Regenerates the architecture map for the new docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jVtT6gPkHFvpiuDLS5rVR
Implements PRD-0017 / ADR-0018: an interactive review-session bridge where an agent opens a session on a page (bare title or pasted wiki URL), the operator queues feedback prompts anchored to Parsoid blocks and cite ids, and the agent long-polls the session to collect them. - sp42-platform: pure review_session core — versioned wire contracts, drain-before-ended feedback semantics, the operator-ended reopen gate, next_step loop etiquette, and the article outline built from Parsoid blocks (the agent's anchor map). - sp42-server: /dev/review/* route group over an in-memory session store with per-session Notify wake-ups; bounded poll waits; all POST routes session+CSRF gated; read-only session inventory listed in the operator endpoint manifest. - sp42-cli: review open/poll/queue/reply/end/sessions subcommands over the bridge-bootstrap pattern, with a re-arming poll loop that keeps stdout reserved for the structured response and narrates waiting on stderr. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012jVtT6gPkHFvpiuDLS5rVR
![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: 31f8f12a7d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else if !self.queued_prompts.is_empty() { | ||
| self.status = ReviewSessionStatus::Feedback; |
There was a problem hiding this comment.
If an operator-ended session receives a normal queue request afterward (for example from a stale browser panel or sp42-cli review queue without --end), this branch moves the session from Ended back to Feedback while leaving ended_by = Operator. The next poll then delivers that prompt with session_ended: false, reopens the session, and leaves an inconsistent Open session that still trips the operator-ended reopen gate; ended sessions should either reject new prompts or preserve ended/final-batch semantics until an explicit reopen.
Useful? React with 👍 / 👎.
| Json(payload): Json<ReviewOpenRequest>, | ||
| ) -> Result<impl IntoResponse, RouteError> { | ||
| let session = require_review_session(&state, &headers).await?; | ||
| let (title, rev_id, outline) = resolve_open_target(&state, &session, &payload).await?; |
There was a problem hiding this comment.
For an existing session the operator already ended, a plain reopen should return the documented 409 guidance, but the route resolves the target and extracts the page outline before it checks gate_reopen. In that scenario, a slow/failing latest-revision or Parsoid lookup can block or return a 5xx instead of the reopen etiquette response, and it performs unnecessary remote reads for a request that should be rejected based only on the existing session key.
Useful? React with 👍 / 👎.
| let wait_ms = if payload.wait_ms == 0 { | ||
| POLL_WAIT_DEFAULT_MS |
There was a problem hiding this comment.
wait_ms: 0 polls
The public ReviewPollRequest contract says wait_ms: 0 returns immediately, but this branch maps zero to the 25s default, and because serde also defaults an omitted field to zero, clients have no way to request a nonblocking status check. Any API caller following the wire contract can unexpectedly hang for the default wait; use an optional field or another sentinel so explicit zero remains immediate while omission can use the default.
Useful? React with 👍 / 👎.
The review session becomes the in-article frontend for the verification report: `sp42-cli review findings` attaches a verify-page report to the session, its findings project onto the outline blocks by cite id (verdict, truncated claim, grounding/unavailable caveat), and the next open returns the annotated outline instead of leaving the operator with detached report text. Revision-gated (409 on mismatch, stale overlay dropped on re-pin), replace-all semantics, unmatched markers surfaced as unanchored_findings. Promotes PRD-0017 open question 2 into the proposal; ADR-0018 gains the marker-projection decision (platform stays citation-agnostic). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2845b00a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let request = ReviewFindingsRequest { | ||
| wiki_id: args.wiki.clone(), | ||
| title: sp42_core::parse_page_target(&args.target).title, | ||
| rev_id: report.rev_id, | ||
| findings: review_finding_markers(&report), |
There was a problem hiding this comment.
When --report points at a verification report for a different page or wiki, this request still uses the CLI --wiki/TARGET and only carries report.rev_id plus projected markers. The server can then only compare the numeric revision, so a mistyped/reused report can attach to the wrong open session when revision IDs collide across wikis, showing unrelated findings in the operator's outline; validate report.wiki_id/report.title against the requested session (or derive the request identity from the report) before posting.
Useful? React with 👍 / 👎.
| entry.session.agent_reply(&payload.text, now_ms); | ||
| Ok(Json(ReviewAckResponse { | ||
| contract_version: REVIEW_SESSION_CONTRACT_VERSION, | ||
| session: entry.session.snapshot(), |
There was a problem hiding this comment.
In the review reply and review poll --agent-reply paths, this appends the agent's text to ReviewSession.chat but the response returns only entry.session.snapshot(), and the new open/sessions response shapes also expose snapshots rather than chat. As a result, after an agent posts a summary, the operator surface has no API response from which to read that chat line, making the documented reply feature effectively invisible.
Useful? React with 👍 / 👎.
…ia-backend-2y8vwx # Conflicts: # docs/domains/references/README.md # docs/platform/architecture.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eec6796962
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if self.rev_id != rev_id { | ||
| self.findings.clear(); | ||
| } | ||
| self.rev_id = rev_id; |
There was a problem hiding this comment.
When an existing session is resumed on a different revision (for example, the operator queues block/ref feedback on rev 42 and the agent runs review open --rev 43 before polling), this path re-pins rev_id and only clears findings; queued_prompts is left intact and the next poll delivers anchors from the old outline against the new revision. Since prompts use the same block ordinals/ref IDs that are considered stale for findings, pending prompts should be drained/refused/cleared when changing revisions to avoid applying feedback to the wrong content.
Useful? React with 👍 / 👎.
…erlay - An ended session refuses new prompts (409 review-session-ended) instead of silently flipping back to feedback around the reopen gate; queued pre-end feedback still delivers with its end flag intact. - The open route checks the reopen gate against the stored session before any remote revision/Parsoid read, so a refused reopen cannot hang or 5xx on slow lookups. - ReviewPollRequest.wait_ms is now optional: omitted means the server default, an explicit 0 is the documented nonblocking status check. - `review findings` validates the report's wiki/title against the target session, so a reused report file cannot overlay another page's outline when revision ids collide. - Agent chat replies surface on the open response (`chat`), so a posted summary is readable by the operator surface instead of write-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21021a4c8a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub fn canonical_key(wiki_id: &str, title: &str) -> String { | ||
| format!("{wiki_id}\u{1f}{title}") |
There was a problem hiding this comment.
When the same MediaWiki page is addressed once by URL and later as a bare title containing underscores, these lines create different store keys (Foo Bar from the URL path vs Foo_Bar from the bare title, since parse_page_target only normalizes URL-derived titles). In that common workflow, review poll/queue/findings reports the session as missing even though MediaWiki treats those titles as the same page; normalize underscores (and other title spelling differences) before building the session key.
Useful? React with 👍 / 👎.
| let Some(entry) = store.get_mut(&key) else { | ||
| return Err(missing_session_error(&payload.wiki_id, &payload.title)); | ||
| }; | ||
| entry.session.end(ReviewEndedBy::Agent, now_ms); |
There was a problem hiding this comment.
If the operator has already ended the session (for example with send-and-end) and the agent later runs review end as cleanup, this call overwrites ended_by with Agent. That removes the operator-ended reopen gate, so a subsequent plain review open is allowed even though the last human action ended the review; an agent end should be a no-op or keep ended_by = Operator for already operator-ended sessions.
Useful? React with 👍 / 👎.
Bare titles previously passed through parse_page_target verbatim, so "Grand_Exemple", "Grand Exemple", and a pasted URL spelled three distinct review sessions for one page. Bare titles now get the wiki-independent half of MediaWiki normalization (underscores are spaces, whitespace runs collapse) — shared as sp42_platform::normalize_title_spacing — and ReviewSession::canonical_key applies the same rule, covering direct API callers. First-letter case is deliberately not folded: case rules are wiki-dependent (Wiktionary is case-sensitive), and merging two distinct pages would be worse than splitting one. ADR-0018 §2 now states the actual normalization instead of the realpath analogy it overstated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
Option C of the two-channels question: the substrates stay separate — the review store keeps its durable drain-exactly-once queue, coordination keeps its live-only no-backfill sync — and they converge at the notification layer only. Every review mutation (open, queue, findings, reply, end) publishes a ReviewSignal, a seventh CoordinationMessage kind carrying just the session snapshot, to the wiki's room with the server sender id (0, never echo-suppressed). The reducer relays it without folding room state: the signal is a doorbell, not the data — panels re-fetch through the gated review routes, so a client-forged signal can only cause a harmless re-fetch, and a room with no operators drops it while late joiners recover over REST. Gives the future browser Review panel live push over the existing WebSocket with no second transport and no WS client in the agent shells. ADR-0018 §8 records the decision; PRD-0017 alternative updated from deferred to rejected-with-convergence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7a023fd6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| take | ||
| }; | ||
|
|
||
| if !matches!(take, ReviewFeedbackTake::Waiting) { |
There was a problem hiding this comment.
When this branch is reached because drain_feedback delivered queued prompts, take_feedback has already removed them and may have changed the session from feedback to open (or cleared the final batch), but the route only logs and returns the poll response. A browser panel that received the earlier ReviewSignal from queueing will keep showing the stale pending count/status until some later mutation such as reply/end/open publishes another signal, so agent delivery is not reflected live even though it mutates the session store.
Useful? React with 👍 / 👎.
- Ending an already-ended session is a no-op: an agent cleanup `review end` after an operator send-and-end kept flipping ended_by to Agent, silently lifting the operator-ended reopen gate. - A poll that delivers feedback now rings the coordination room with the post-drain snapshot, so a panel's pending badge does not sit on the queueing signal's stale count until some later mutation. Codex's third comment (canonicalize titles before keying) reviewed the pre-30bbfb0 code and was already fixed by that commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4d6cbc1d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Ok(Json(ReviewAckResponse { | ||
| contract_version: REVIEW_SESSION_CONTRACT_VERSION, | ||
| session: snapshot, | ||
| })) |
There was a problem hiding this comment.
When this acknowledgement is returned after replying to an operator-ended session, the agent's text is stored but only a snapshot comes back; the only response that now exposes chat is review open, and that route returns the operator-ended 409 unless the caller passes reopen=true and mutates the session. Fresh evidence after the earlier comment is that ReviewOpenResponse now includes chat, but final updates written after send-and-end remain inaccessible to the operator without reopening, despite the poll next_step telling the agent to deliver remaining updates in chat.
Useful? React with 👍 / 👎.
…ia-backend-2y8vwx # Conflicts: # crates/sp42-cli/src/main.rs # docs/platform/architecture.md
origin/main does not compile: PR #153 added SkippedReason::UnresolvedShortCite while PR #154 added an exhaustive match over SkippedReason in ga_appendix.rs; they merged independently and the semantic conflict landed on main. This adds the missing arm (with disclosed-not-guessed copy) so this branch builds; main needs the same one-arm fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB2VfS9GXyTzVw3FayQZxf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3acb9b1305
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| (snapshot, Vec::new(), Vec::new()) | ||
| }; | ||
| drop(store); | ||
| publish_review_signal(&state, &session).await; |
There was a problem hiding this comment.
When a live panel follows the doorbell pattern and re-fetches /dev/review/open after a ReviewSignal (the only other review read route I found is /dev/review/sessions, which returns only summaries), this unconditional publish sends a fresh ReviewSignal for the re-fetch itself. That makes signal → open → signal repeat indefinitely and re-runs the Parsoid outline extraction on each cycle; publish here only for actual create/re-pin/reopen mutations or add a read-only detail route for panel refreshes.
Useful? React with 👍 / 👎.
2 participants
https://claude.ai/code/session_012jVtT6gPkHFvpiuDLS5rVR