diff --git a/crates/sp42-assessment/src/copy.rs b/crates/sp42-assessment/src/copy.rs index b82715ba..ca4b9a22 100644 --- a/crates/sp42-assessment/src/copy.rs +++ b/crates/sp42-assessment/src/copy.rs @@ -114,6 +114,11 @@ pub const SKIPPED_BOOK_UNRESOLVED: &str = /// problem) — the book may well have a record; nothing is implied about it. pub const SKIPPED_BOOK_LOOKUP_FAILED: &str = "cites a book whose catalog lookup did not complete — the tool could not reach the catalog; nothing is implied about the book"; +/// Skip reason: a short cite ({{sfn}}-style) whose bibliography entry could +/// not be resolved to a usable identifier — never guessed, disclosed. +pub const SKIPPED_SHORT_CITE_UNRESOLVED: &str = + "is a short cite the tool could not resolve to a bibliography entry with a usable identifier"; + /// Books consulted section heading. pub const BUCKET_BOOKS_CONSULTED: &str = "==== Books consulted ===="; diff --git a/crates/sp42-assessment/src/ga_appendix.rs b/crates/sp42-assessment/src/ga_appendix.rs index beed90c0..a244f550 100644 --- a/crates/sp42-assessment/src/ga_appendix.rs +++ b/crates/sp42-assessment/src/ga_appendix.rs @@ -321,6 +321,9 @@ fn render_skipped_section(output: &mut String, report: &sp42_citation::PageVerif crate::copy::SKIPPED_BOOK_UNRESOLVED } } + sp42_citation::SkippedReason::UnresolvedShortCite => { + crate::copy::SKIPPED_SHORT_CITE_UNRESOLVED + } }); // Which book: identifiers (and cited page) make multiple books // under one ref distinguishable (Codex round 4, PR 154). diff --git a/crates/sp42-citation/src/citation/page.rs b/crates/sp42-citation/src/citation/page.rs index a1f71b76..3d10052f 100644 --- a/crates/sp42-citation/src/citation/page.rs +++ b/crates/sp42-citation/src/citation/page.rs @@ -203,6 +203,52 @@ pub struct PageVerificationReport { pub stats: PageVerificationStats, } +/// Project a page report's findings into review-session overlay markers +/// (PRD-0017): one marker per finding, joined to the article by the +/// finding's `ref_id` — the same anchor coordinate review prompts use — so +/// a review surface can show the report's problems in the context of the +/// article instead of as detached text. Verdicts keep their wire labels; +/// claims are truncated to the outline display limit; grounding caveats and +/// unavailable reasons ride along as `detail`. Findings without a `ref_id` +/// (the standalone single-claim path) cannot anchor and are skipped. +#[must_use] +pub fn review_finding_markers( + report: &PageVerificationReport, +) -> Vec { + report + .findings + .iter() + .filter(|finding| !finding.ref_id.is_empty()) + .map(|finding| sp42_platform::ReviewFindingMarker { + ref_id: finding.ref_id.clone(), + verdict: finding.verdict.as_wire().to_string(), + claim: sp42_platform::truncate_outline_text(&finding.claim), + detail: finding_marker_detail(finding), + }) + .collect() +} + +/// The marker's short qualifier: the unavailable reason for an abstention, +/// or the grounding caveat for a support judgment that did not locate +/// verbatim (SP42#25 layer 6 — surfaced, never silently upgraded). +fn finding_marker_detail(finding: &CitationFinding) -> Option { + if finding.verdict == CitationVerdict::SourceUnavailable { + return finding + .source_unavailable_reason + .map(|reason| format!("source {}", reason.as_str())); + } + match finding.grounding_status { + crate::citation::verify::GroundingStatus::Located + | crate::citation::verify::GroundingStatus::NotApplicable => None, + crate::citation::verify::GroundingStatus::LocatedFuzzy => { + Some("support located by fuzzy match only".to_string()) + } + crate::citation::verify::GroundingStatus::Unlocated => { + Some("support quote not located in source (unverified)".to_string()) + } + } +} + /// Try archive fallbacks if the primary URL came back `SourceUnavailable`. /// Returns the original outcome if no archives work or if no archives exist. async fn try_archive_fallback( @@ -1004,6 +1050,52 @@ mod orchestrator_tests { } } + #[test] + fn review_finding_markers_project_the_report_onto_review_anchors() { + let judged = |level| CitationVerdict::Judged(level); + let mut unlocated = mk_finding("r_unlocated", 1, judged(SupportLevel::Supported)); + unlocated.grounding_status = GroundingStatus::Unlocated; + let mut dead = mk_finding("r_dead", 2, CitationVerdict::SourceUnavailable); + dead.source_unavailable_reason = Some(SourceUnavailableReason::Unreachable); + let mut long_claim = mk_finding("r_long", 3, judged(SupportLevel::Partial)); + long_claim.claim = "x".repeat(500); + // The standalone single-claim path has no page ref: nothing to anchor to. + let standalone = mk_finding("", 4, judged(SupportLevel::NotSupported)); + let report = PageVerificationReport { + wiki_id: "enwiki".into(), + title: "Cats".into(), + rev_id: 7, + findings: vec![ + mk_finding("r_ok", 0, judged(SupportLevel::Supported)), + unlocated, + dead, + long_claim, + standalone, + ], + skipped: Vec::new(), + extraction_failures: Vec::new(), + book_resolutions: Vec::new(), + stats: PageVerificationStats::default(), + }; + + let markers = super::review_finding_markers(&report); + + assert_eq!(markers.len(), 4, "ref-less standalone finding is skipped"); + assert_eq!(markers[0].ref_id, "r_ok"); + assert_eq!(markers[0].verdict, "supported"); + assert_eq!(markers[0].detail, None); + assert_eq!( + markers[1].detail.as_deref(), + Some("support quote not located in source (unverified)") + ); + assert_eq!(markers[2].verdict, "source_unavailable"); + assert_eq!(markers[2].detail.as_deref(), Some("source unreachable")); + assert!( + markers[3].claim.chars().count() <= 200, + "claims are truncated to the outline display limit" + ); + } + #[test] fn apply_reverified_finding_moves_verdict_and_updates_tallies() { let judged = |level| CitationVerdict::Judged(level); diff --git a/crates/sp42-citation/src/citation/urls.rs b/crates/sp42-citation/src/citation/urls.rs index 61f3ad47..430f3a91 100644 --- a/crates/sp42-citation/src/citation/urls.rs +++ b/crates/sp42-citation/src/citation/urls.rs @@ -12,6 +12,7 @@ use regex::Regex; use url::Url; use crate::errors::CitationVerificationError; +use sp42_platform::wikitext_editor::normalize_title_spacing; /// Accepted wiki language/site codes — the SSRF guard before host interpolation. static WIKI_CODE: LazyLock = LazyLock::new(|| { @@ -166,9 +167,14 @@ pub struct PageTarget { /// - `https://…/w/index.php?title=Foo&oldid=123` → title `Foo`, `rev_id` `123`. /// /// Underscores become spaces and percent-escapes are decoded, matching `MediaWiki` -/// title normalization. Anything that is not an `http(s)` URL is treated as a -/// bare title verbatim. The wiki the URL points at is *not* inspected — the caller -/// chooses the wiki separately, and a host mismatch is theirs to reconcile. +/// title normalization. A bare (non-URL) title gets the same spacing +/// normalization — underscores to spaces, whitespace runs collapsed — but no +/// percent-decoding, since a literal `%` is valid title text. First-letter +/// case is deliberately *not* folded: case rules are wiki-dependent +/// (Wiktionary titles are case-sensitive), and conflating two distinct pages +/// would be worse than treating one page as two. The wiki the URL points at +/// is *not* inspected — the caller chooses the wiki separately, and a host +/// mismatch is theirs to reconcile. #[must_use] pub fn parse_page_target(input: &str) -> PageTarget { let trimmed = input.trim(); @@ -207,20 +213,15 @@ pub fn parse_page_target(input: &str) -> PageTarget { } PageTarget { - title: trimmed.to_string(), + title: normalize_title_spacing(trimmed), rev_id: 0, } } -/// Normalize a title extracted from a URL: percent-decode, then map underscores -/// to spaces (`MediaWiki` treats them as equivalent and the action API expects -/// spaces). +/// Normalize a title extracted from a URL: percent-decode, then apply the +/// spacing normalization. fn normalize_title(raw: &str) -> String { - percent_decode_str(raw) - .decode_utf8_lossy() - .replace('_', " ") - .trim() - .to_string() + normalize_title_spacing(&percent_decode_str(raw).decode_utf8_lossy()) } #[cfg(test)] @@ -237,6 +238,21 @@ mod tests { assert_eq!(target.rev_id, 0); } + #[test] + fn parse_page_target_normalizes_bare_title_spacing() { + // Underscores are spaces in MediaWiki, and runs of either collapse + // to one — a bare title must not spell a different page (or review + // session) than the same title pasted as a URL. + assert_eq!( + parse_page_target("Grand_Exemple").title, + "Grand Exemple", + "underscore spelling matches the URL spelling" + ); + assert_eq!(parse_page_target("A __ B C").title, "A B C"); + // No percent-decoding for bare titles: a literal `%` is valid text. + assert_eq!(parse_page_target("100%_Design").title, "100% Design"); + } + #[test] fn parse_page_target_unwraps_a_wiki_url() { // The reported failure: a pasted /wiki/ URL with an underscore subpage. diff --git a/crates/sp42-citation/src/lib.rs b/crates/sp42-citation/src/lib.rs index 49477a0e..c78881a4 100644 --- a/crates/sp42-citation/src/lib.rs +++ b/crates/sp42-citation/src/lib.rs @@ -58,8 +58,8 @@ pub use citation::openlibrary_apply::{ }; pub use citation::page::{ PageVerificationReport, PageVerificationRequest, PageVerificationStats, ReverifyFindingRequest, - ReverifyFindingResponse, apply_reverified_finding, select_reverify_use_site, - verify_one_use_site, verify_page, + ReverifyFindingResponse, apply_reverified_finding, review_finding_markers, + select_reverify_use_site, verify_one_use_site, verify_page, }; pub use citation::parsing::{ ParsedVerdict, canonicalize_verdict, parse_repair_response, parse_verdict_response, diff --git a/crates/sp42-cli/src/main.rs b/crates/sp42-cli/src/main.rs index e34a7828..0574ff03 100644 --- a/crates/sp42-cli/src/main.rs +++ b/crates/sp42-cli/src/main.rs @@ -11,10 +11,14 @@ use sp42_core::routes as route_contracts; use sp42_core::{ CitationFinding, CitationVerificationRequest, CitoidMetadata, ClaimContext, DevAuthBootstrapRequest, DevAuthSessionStatus, FetchedSource, GroundingStatus, - PageVerificationReport, PageVerificationRequest, QueuedEdit, SessionActionExecutionRequest, + PageVerificationReport, PageVerificationRequest, QueuedEdit, ReviewAckResponse, ReviewAnchor, + ReviewEndRequest, ReviewFindingsRequest, ReviewFindingsResponse, ReviewOpenRequest, + ReviewOpenResponse, ReviewPollRequest, ReviewPollResponse, ReviewPollStatus, ReviewPrompt, + ReviewPromptKind, ReviewQueueRequest, ReviewQueueResponse, ReviewReplyRequest, + ReviewSessionSnapshot, ReviewSessionsResponse, SessionActionExecutionRequest, SessionActionExecutionResponse, SessionActionKind, SystemClock, VerificationOutcome, VerifyOptions as CoreVerifyOptions, build_dev_auth_bootstrap_request, locate_quote, - locate_quote_fuzzy, parse_dev_auth_status, verify_citation_use_site, + locate_quote_fuzzy, parse_dev_auth_status, review_finding_markers, verify_citation_use_site, }; use sp42_devtools::{ DEV_PREVIEW_SAMPLE_EVENTS, DEV_PREVIEW_WIKI_ID, DevContextOptions, DevWorkbenchOptions, @@ -261,6 +265,8 @@ enum Command { Batch(BatchArgs), /// Preview or apply bare-URL citation repairs (PRD-0008). BareUrl(BareUrlArgs), + /// Interactive review sessions: open a page, wait for operator feedback (PRD-0017). + Review(ReviewArgs), /// Dev / operator queue views; reads the event payload from STDIN. Preview(PreviewArgs), } @@ -370,6 +376,157 @@ struct RenderReportArgs { fmt: PageReportFormatArg, } +#[derive(Debug, Args)] +struct ReviewArgs { + #[command(subcommand)] + action: ReviewAction, +} + +#[derive(Debug, Subcommand)] +enum ReviewAction { + /// Open or resume a review session on a page (bare title or pasted wiki URL). + Open(ReviewOpenArgs), + /// Wait for operator feedback on an open session; re-arms until feedback or end. + Poll(ReviewPollArgs), + /// Queue operator feedback from the command line (dev/test surface). + Queue(ReviewQueueArgs), + /// Attach a verify-page report to the session so its findings overlay + /// the article outline (the report's in-article frontend). + Findings(ReviewFindingsArgs), + /// Send an agent chat reply to the operator surface. + Reply(ReviewReplyArgs), + /// End a session as the agent (a plain reopen stays allowed). + End(ReviewEndArgs), + /// List review sessions on the local server. + Sessions(ReviewSessionsArgs), +} + +#[derive(Debug, Args)] +struct ReviewOpenArgs { + /// Page target: bare title or pasted wiki URL (oldid URLs pin the revision). + target: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Revision id. Omit for the latest revision (the server resolves it). + #[arg(long)] + rev: Option, + /// Resume a session the operator explicitly ended (only when they ask). + #[arg(long)] + reopen: bool, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewPollArgs { + /// Page target: bare title or pasted wiki URL. + target: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Chat reply shown to the operator before the wait starts. + #[arg(long)] + agent_reply: Option, + /// Return after one bounded server wait instead of re-arming until feedback. + #[arg(long)] + once: bool, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewQueueArgs { + /// Page target: bare title or pasted wiki URL. + target: String, + /// The feedback prompt to queue. + #[arg(long)] + message: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Anchor the prompt to this block ordinal. + #[arg(long)] + block: Option, + /// Anchor the prompt to this cite id (e.g. cite_ref-x_2-0). + #[arg(long)] + ref_id: Option, + /// Anchor the prompt to this verbatim selected text. + #[arg(long)] + selected_text: Option, + /// Queue and end the session in one action ("send & end"). + #[arg(long)] + end: bool, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewFindingsArgs { + /// Page target: bare title or pasted wiki URL. + target: String, + /// Path to a `verify-page --format json` report, or `-` for stdin. + #[arg(long)] + report: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewReplyArgs { + /// Page target: bare title or pasted wiki URL. + target: String, + /// The reply text. + #[arg(long)] + message: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewEndArgs { + /// Page target: bare title or pasted wiki URL. + target: String, + /// Target wiki id. + #[arg(long, default_value = BARE_URL_DEFAULT_WIKI)] + wiki: String, + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + +#[derive(Debug, Args)] +struct ReviewSessionsArgs { + /// Local server base URL. + #[arg(long, default_value = LOCAL_SERVER_BASE_URL)] + bridge_base_url: String, + #[command(flatten)] + fmt: FormatArg, +} + #[derive(Debug, Args)] struct LocateProbeArgs { /// The quote to locate within the STDIN source body. @@ -739,6 +896,7 @@ fn dispatch(command: Command) -> Result { models: args.models, }), Command::BareUrl(args) => dispatch_bare_url(args.action), + Command::Review(args) => dispatch_review(args.action), Command::Preview(args) => dispatch_preview(args), } } @@ -3191,6 +3349,453 @@ async fn execute_bare_url_via_bridge( }) } +/// An authenticated bridge session: the reqwest client plus the cookie and +/// CSRF token every gated review request must carry (ADR-0002). +struct BridgeSession { + client: reqwest::Client, + cookie: String, + csrf_token: String, +} + +/// Bootstrap the local dev-auth bridge and capture the session cookie and +/// CSRF token — the shared preamble of every gated bridge call. +async fn bootstrap_bridge_session(base_url: &str) -> Result { + let client = reqwest::Client::builder() + .user_agent(sp42_core::branding::USER_AGENT) + .build() + .map_err(|error| format!("bridge client failed to build: {error}"))?; + let bootstrap_request = + build_dev_auth_bootstrap_request(base_url, &DevAuthBootstrapRequest::default()) + .map_err(|error| error.to_string())?; + let bootstrap_response = execute_local_http_request(&client, bootstrap_request).await?; + let bootstrap = + parse_dev_auth_status(&bootstrap_response.body).map_err(|error| error.to_string())?; + if !bootstrap.authenticated { + return Err("bridge bootstrap did not produce an authenticated session".to_string()); + } + let cookie = session_cookie_from_headers(&bootstrap_response.headers) + .ok_or_else(|| "bridge bootstrap did not set a session cookie".to_string())?; + let csrf_token = bootstrap + .csrf_token + .ok_or_else(|| "bridge bootstrap did not return a CSRF token".to_string())?; + Ok(BridgeSession { + client, + cookie, + csrf_token, + }) +} + +/// POST a JSON body to a gated review route and parse the JSON response. +async fn post_review_route( + bridge: &BridgeSession, + base_url: &str, + path: &str, + request: &Request, +) -> Result +where + Request: serde::Serialize, + Response: serde::de::DeserializeOwned, +{ + let response = bridge + .client + .post(format!("{base_url}{path}")) + .header(COOKIE, bridge.cookie.as_str()) + .header( + route_contracts::CSRF_HEADER_NAME, + bridge.csrf_token.as_str(), + ) + .json(request) + .send() + .await + .map_err(|error| format!("review request to {path} failed: {error}"))?; + let status = response.status(); + if !status.is_success() { + // Keep the server's JSON detail: refusals like the operator-ended + // reopen gate carry their etiquette guidance in the body. + let body = response.text().await.unwrap_or_default(); + return Err(format!( + "review request to {path} failed: HTTP {status}: {body}" + )); + } + response + .json::() + .await + .map_err(|error| format!("review response from {path} was invalid: {error}")) +} + +/// Route a `review` subcommand to its runner on a fresh Tokio runtime +/// (`reqwest` needs a reactor; mirrors `render_verify_page`). +fn dispatch_review(action: ReviewAction) -> Result { + let runtime = tokio::runtime::Runtime::new() + .map_err(|error| format!("failed to start async runtime: {error}"))?; + match action { + ReviewAction::Open(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_open(&args))?; + render_review_output(format, &response, render_review_open_text(&response)) + } + ReviewAction::Poll(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_poll(&args))?; + render_review_output(format, &response, render_review_poll_text(&response)) + } + ReviewAction::Queue(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_queue(&args))?; + let text = format!( + "queued {} prompt(s)\n{}", + response.queued, + render_review_snapshot_line(&response.session) + ); + render_review_output(format, &response, text) + } + ReviewAction::Findings(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_findings(&args))?; + let text = format!( + "attached {} finding marker(s)\n{}", + response.attached, + render_review_snapshot_line(&response.session) + ); + render_review_output(format, &response, text) + } + ReviewAction::Reply(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_reply(&args))?; + let text = format!( + "reply delivered\n{}", + render_review_snapshot_line(&response.session) + ); + render_review_output(format, &response, text) + } + ReviewAction::End(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_end(&args))?; + let text = format!( + "session ended by agent\n{}", + render_review_snapshot_line(&response.session) + ); + render_review_output(format, &response, text) + } + ReviewAction::Sessions(args) => { + let format = args.fmt.format; + let response = runtime.block_on(run_review_sessions(&args))?; + render_review_output(format, &response, render_review_sessions_text(&response)) + } + } +} + +async fn run_review_open(args: &ReviewOpenArgs) -> Result { + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let request = ReviewOpenRequest { + wiki_id: args.wiki.clone(), + target: args.target.clone(), + rev_id: args.rev.unwrap_or(0), + reopen: args.reopen, + }; + post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_OPEN_PATH, + &request, + ) + .await +} + +/// Wait for operator feedback. Each request is a bounded server-side wait; +/// the loop re-arms until feedback, an end, or a missing session, so from +/// the agent's side this behaves like one long poll. Waiting narration goes +/// to stderr; stdout stays reserved for the final structured response. +async fn run_review_poll(args: &ReviewPollArgs) -> Result { + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let title = sp42_core::parse_page_target(&args.target).title; + if let Some(reply) = &args.agent_reply { + let request = ReviewReplyRequest { + wiki_id: args.wiki.clone(), + title: title.clone(), + text: reply.clone(), + }; + let _: ReviewAckResponse = post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_REPLY_PATH, + &request, + ) + .await?; + } + let request = ReviewPollRequest { + wiki_id: args.wiki.clone(), + title: title.clone(), + // Omitted = the server's default bounded wait; the CLI re-arms + // between waits. (An explicit 0 is a nonblocking status check.) + wait_ms: None, + }; + loop { + let response: ReviewPollResponse = post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_POLL_PATH, + &request, + ) + .await?; + if args.once || response.status != ReviewPollStatus::Waiting { + return Ok(response); + } + eprintln!( + "review poll: still waiting for operator feedback on {title} \ + (re-arming; queued feedback is never lost)" + ); + } +} + +/// Build the queue prompt from CLI anchor flags: `--selected-text` makes a +/// text prompt, `--block`/`--ref-id` a block prompt, neither a free-form +/// message. Anchored prompts require `--block` so the anchor is complete. +fn build_review_queue_prompt(args: &ReviewQueueArgs) -> Result { + let anchored = args.ref_id.is_some() || args.selected_text.is_some(); + let Some(block_ordinal) = args.block else { + if anchored { + return Err("--ref-id/--selected-text need --block to anchor the prompt".to_string()); + } + return Ok(ReviewPrompt { + kind: ReviewPromptKind::Message, + prompt: args.message.clone(), + anchor: None, + }); + }; + let kind = if args.selected_text.is_some() { + ReviewPromptKind::Text + } else { + ReviewPromptKind::Block + }; + Ok(ReviewPrompt { + kind, + prompt: args.message.clone(), + anchor: Some(ReviewAnchor { + block_ordinal, + ref_id: args.ref_id.clone(), + selected_text: args.selected_text.clone(), + }), + }) +} + +async fn run_review_queue(args: &ReviewQueueArgs) -> Result { + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let request = ReviewQueueRequest { + wiki_id: args.wiki.clone(), + title: sp42_core::parse_page_target(&args.target).title, + prompts: vec![build_review_queue_prompt(args)?], + end_session: args.end, + }; + post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_PROMPTS_PATH, + &request, + ) + .await +} + +/// Read a `verify-page --format json` report from a file or stdin (`-`). +fn read_verification_report(path: &str) -> Result { + let raw = if path == "-" { + let mut buffer = String::new(); + std::io::Read::read_to_string(&mut std::io::stdin(), &mut buffer) + .map_err(|error| format!("failed to read report from stdin: {error}"))?; + buffer + } else { + std::fs::read_to_string(path) + .map_err(|error| format!("failed to read report {path}: {error}"))? + }; + serde_json::from_str(&raw) + .map_err(|error| format!("report is not a verify-page JSON report: {error}")) +} + +/// Refuse a report whose identity does not match the requested session: the +/// server can only compare revisions, and revision ids can collide across +/// wikis, so a mistyped or reused report file could otherwise overlay +/// unrelated findings onto the operator's outline. +fn check_report_identity( + report: &PageVerificationReport, + wiki: &str, + title: &str, +) -> Result<(), String> { + // MediaWiki treats underscores and spaces in titles as the same page. + let canonical = |t: &str| t.replace('_', " "); + if report.wiki_id != wiki || canonical(&report.title) != canonical(title) { + return Err(format!( + "the report describes {}:{} but the target session is {wiki}:{title}; \ + pass the report produced for this page", + report.wiki_id, report.title + )); + } + Ok(()) +} + +/// Attach a verify-page report to the review session: project its findings +/// onto review anchors (ref ids) and post them, pinned to the report's +/// revision so a stale report cannot overlay a newer session. +async fn run_review_findings(args: &ReviewFindingsArgs) -> Result { + let report = read_verification_report(&args.report)?; + let title = sp42_core::parse_page_target(&args.target).title; + check_report_identity(&report, &args.wiki, &title)?; + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let request = ReviewFindingsRequest { + wiki_id: args.wiki.clone(), + title, + rev_id: report.rev_id, + findings: review_finding_markers(&report), + }; + post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_FINDINGS_PATH, + &request, + ) + .await +} + +async fn run_review_reply(args: &ReviewReplyArgs) -> Result { + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let request = ReviewReplyRequest { + wiki_id: args.wiki.clone(), + title: sp42_core::parse_page_target(&args.target).title, + text: args.message.clone(), + }; + post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_REPLY_PATH, + &request, + ) + .await +} + +async fn run_review_end(args: &ReviewEndArgs) -> Result { + let bridge = bootstrap_bridge_session(&args.bridge_base_url).await?; + let request = ReviewEndRequest { + wiki_id: args.wiki.clone(), + title: sp42_core::parse_page_target(&args.target).title, + }; + post_review_route( + &bridge, + &args.bridge_base_url, + route_contracts::DEV_REVIEW_END_PATH, + &request, + ) + .await +} + +async fn run_review_sessions(args: &ReviewSessionsArgs) -> Result { + let client = reqwest::Client::builder() + .user_agent(sp42_core::branding::USER_AGENT) + .build() + .map_err(|error| format!("bridge client failed to build: {error}"))?; + let url = format!( + "{}{}", + args.bridge_base_url, + route_contracts::DEV_REVIEW_SESSIONS_PATH + ); + let response = client + .get(&url) + .send() + .await + .map_err(|error| format!("review sessions request failed: {error}"))? + .error_for_status() + .map_err(|error| format!("review sessions request failed: {error}"))?; + response + .json::() + .await + .map_err(|error| format!("review sessions payload was invalid: {error}")) +} + +/// Shared review output shaping: JSON is the machine contract; text and +/// markdown share the same line-oriented rendering. +fn render_review_output( + format: OutputFormat, + value: &T, + text: String, +) -> Result { + match format { + OutputFormat::Json => { + serde_json::to_string_pretty(value).map_err(|error| error.to_string()) + } + OutputFormat::Text => Ok(text), + OutputFormat::Markdown => Ok(format!("## Review session\n\n```\n{text}\n```")), + } +} + +fn render_review_snapshot_line(session: &ReviewSessionSnapshot) -> String { + let ended = session.ended_by.map_or(String::new(), |by| { + format!(", ended by {by:?}").to_lowercase() + }); + format!( + "session: {} ({}) rev {} — {:?}, {} pending prompt(s){}", + session.title, + session.wiki_id, + session.rev_id, + session.status, + session.pending_prompts, + ended + ) +} + +fn render_review_open_text(response: &ReviewOpenResponse) -> String { + let unanchored = if response.unanchored_findings.is_empty() { + String::new() + } else { + format!( + "\nunanchored findings: {} (attached but matching no outline block)", + response.unanchored_findings.len() + ) + }; + format!( + "{}\noutline: {} block(s), {} finding(s) attached{}\nnext: {}", + render_review_snapshot_line(&response.session), + response.outline.len(), + response.session.findings, + unanchored, + response.next_step + ) +} + +fn render_review_prompt_line(prompt: &ReviewPrompt) -> String { + let anchor = prompt.anchor.as_ref().map_or(String::new(), |anchor| { + let ref_part = anchor + .ref_id + .as_deref() + .map_or(String::new(), |ref_id| format!(", ref {ref_id}")); + let text_part = anchor + .selected_text + .as_deref() + .map_or(String::new(), |text| format!(", text \"{text}\"")); + format!(" (block {}{ref_part}{text_part})", anchor.block_ordinal) + }); + format!("- [{:?}] {}{anchor}", prompt.kind, prompt.prompt).to_string() +} + +fn render_review_poll_text(response: &ReviewPollResponse) -> String { + let mut lines = vec![format!("status: {:?}", response.status).to_lowercase()]; + lines.extend(response.prompts.iter().map(render_review_prompt_line)); + if let Some(ended_by) = response.ended_by { + lines.push(format!("ended by: {ended_by:?}").to_lowercase()); + } + lines.push(format!("next: {}", response.next_step)); + lines.join("\n") +} + +fn render_review_sessions_text(response: &ReviewSessionsResponse) -> String { + if response.sessions.is_empty() { + return "no review sessions".to_string(); + } + response + .sessions + .iter() + .map(render_review_snapshot_line) + .collect::>() + .join("\n") +} + async fn execute_local_http_request( client: &reqwest::Client, request: HttpRequest, @@ -3531,10 +4136,13 @@ mod tests { use super::{ ActionKind, BareUrlAction, BareUrlCliMode, BareUrlCliOptions, BareUrlExecuteReport, Cli, - CliOptions, Command, ContextPreviewOptions, LOCAL_SERVER_BASE_URL, OutputFormat, - PageReportFormat, ShellMode, VerifyPageCliOptions, WorkbenchOptions, render_action_preview, - render_backlog_preview, render_bare_url_execute, render_bare_url_proposals, - render_context_preview, render_coordination_preview, render_parity_report, render_queue, + CliOptions, Command, ContextPreviewOptions, FormatArg, LOCAL_SERVER_BASE_URL, OutputFormat, + PageReportFormat, PageVerificationReport, ReviewAction, ReviewAnchor, ReviewPollResponse, + ReviewPollStatus, ReviewPrompt, ReviewPromptKind, ReviewQueueArgs, ShellMode, + VerifyPageCliOptions, WorkbenchOptions, build_review_queue_prompt, check_report_identity, + read_verification_report, render_action_preview, render_backlog_preview, + render_bare_url_execute, render_bare_url_proposals, render_context_preview, + render_coordination_preview, render_parity_report, render_queue, render_review_poll_text, render_scenario_report, render_session_digest, render_stream_preview, render_workbench, run_render_report, select_bare_url_proposal, server_report_lines, }; @@ -4607,4 +5215,190 @@ mod tests { assert!(error.contains("no bare-URL proposal for ordinal 1")); assert!(error.contains("declined: []")); } + + #[test] + fn review_open_args_parse_target_wiki_rev_and_reopen() { + let cli = Cli::parse_from([ + "sp42-cli", + "review", + "open", + "https://en.wikipedia.org/wiki/Example", + "--wiki", + "enwiki", + "--rev", + "5", + "--reopen", + ]); + let Command::Review(args) = cli.command else { + panic!("expected review command"); + }; + let ReviewAction::Open(open) = args.action else { + panic!("expected open action"); + }; + assert_eq!(open.target, "https://en.wikipedia.org/wiki/Example"); + assert_eq!(open.wiki, "enwiki"); + assert_eq!(open.rev, Some(5)); + assert!(open.reopen); + assert_eq!(open.bridge_base_url, LOCAL_SERVER_BASE_URL); + } + + #[test] + fn parses_review_findings_action() { + let cli = Cli::parse_from([ + "sp42-cli", + "review", + "findings", + "Exemple", + "--report", + "report.json", + "--wiki", + "frwiki", + ]); + let Command::Review(args) = cli.command else { + panic!("expected review command"); + }; + let ReviewAction::Findings(findings) = args.action else { + panic!("expected findings action"); + }; + assert_eq!(findings.target, "Exemple"); + assert_eq!(findings.report, "report.json"); + assert_eq!(findings.wiki, "frwiki"); + } + + #[test] + fn read_verification_report_rejects_non_report_json() { + let dir = + std::env::temp_dir().join(format!("sp42-cli-review-findings-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("temp dir should create"); + let path = dir.join("not-a-report.json"); + std::fs::write(&path, "{\"outcome\": true}").expect("temp file should write"); + + let error = read_verification_report(path.to_str().expect("utf-8 path")) + .expect_err("non-report JSON should be rejected"); + assert!(error.contains("not a verify-page JSON report")); + } + + #[test] + fn report_identity_check_matches_page_not_just_revision() { + let report = |wiki: &str, title: &str| PageVerificationReport { + wiki_id: wiki.to_string(), + title: title.to_string(), + rev_id: 42, + findings: Vec::new(), + skipped: Vec::new(), + extraction_failures: Vec::new(), + book_resolutions: Vec::new(), + stats: sp42_core::PageVerificationStats::default(), + }; + + assert!(check_report_identity(&report("frwiki", "Exemple"), "frwiki", "Exemple").is_ok()); + // Underscore and space spellings are the same MediaWiki title. + assert!( + check_report_identity( + &report("frwiki", "Grand_Exemple"), + "frwiki", + "Grand Exemple" + ) + .is_ok() + ); + + let wrong_wiki = check_report_identity(&report("enwiki", "Exemple"), "frwiki", "Exemple") + .expect_err("a report for another wiki must not attach"); + assert!(wrong_wiki.contains("enwiki:Exemple")); + assert!( + check_report_identity(&report("frwiki", "Autre"), "frwiki", "Exemple").is_err(), + "a report for another page must not attach" + ); + } + + #[test] + fn review_queue_prompt_builds_each_anchor_shape() { + let base = ReviewQueueArgs { + target: "Exemple".to_string(), + message: "tighten this".to_string(), + wiki: "frwiki".to_string(), + block: None, + ref_id: None, + selected_text: None, + end: false, + bridge_base_url: LOCAL_SERVER_BASE_URL.to_string(), + fmt: FormatArg { + format: OutputFormat::Text, + }, + }; + + let message = build_review_queue_prompt(&base).expect("message prompt should build"); + assert_eq!(message.kind, ReviewPromptKind::Message); + assert!(message.anchor.is_none()); + + let block = build_review_queue_prompt(&ReviewQueueArgs { + block: Some(3), + ref_id: Some("cite_ref-a_1-0".to_string()), + ..clone_review_queue_args(&base) + }) + .expect("block prompt should build"); + assert_eq!(block.kind, ReviewPromptKind::Block); + let anchor = block.anchor.expect("block prompt should carry an anchor"); + assert_eq!(anchor.block_ordinal, 3); + assert_eq!(anchor.ref_id.as_deref(), Some("cite_ref-a_1-0")); + + let text = build_review_queue_prompt(&ReviewQueueArgs { + block: Some(2), + selected_text: Some("the quote".to_string()), + ..clone_review_queue_args(&base) + }) + .expect("text prompt should build"); + assert_eq!(text.kind, ReviewPromptKind::Text); + + let missing_block = build_review_queue_prompt(&ReviewQueueArgs { + ref_id: Some("cite_ref-a_1-0".to_string()), + ..clone_review_queue_args(&base) + }) + .expect_err("an anchored prompt without --block must refuse"); + assert!(missing_block.contains("--block")); + } + + fn clone_review_queue_args(args: &ReviewQueueArgs) -> ReviewQueueArgs { + ReviewQueueArgs { + target: args.target.clone(), + message: args.message.clone(), + wiki: args.wiki.clone(), + block: args.block, + ref_id: args.ref_id.clone(), + selected_text: args.selected_text.clone(), + end: args.end, + bridge_base_url: args.bridge_base_url.clone(), + fmt: FormatArg { + format: OutputFormat::Text, + }, + } + } + + #[test] + fn review_poll_text_lists_prompts_and_next_step() { + let response = ReviewPollResponse { + contract_version: 1, + status: ReviewPollStatus::Feedback, + prompts: vec![ReviewPrompt { + kind: ReviewPromptKind::Text, + prompt: "check this quote".to_string(), + anchor: Some(ReviewAnchor { + block_ordinal: 3, + ref_id: Some("cite_ref-x_2-0".to_string()), + selected_text: Some("the quote".to_string()), + }), + }], + session_ended: false, + ended_by: None, + next_step: "apply the feedback".to_string(), + }; + + let text = render_review_poll_text(&response); + + assert!(text.contains("status: feedback")); + assert!(text.contains("check this quote")); + assert!(text.contains("block 3")); + assert!(text.contains("cite_ref-x_2-0")); + assert!(text.contains("next: apply the feedback")); + } } diff --git a/crates/sp42-coordination/src/codec.rs b/crates/sp42-coordination/src/codec.rs index 9c743e59..d3dba8e2 100644 --- a/crates/sp42-coordination/src/codec.rs +++ b/crates/sp42-coordination/src/codec.rs @@ -46,6 +46,19 @@ mod tests { assert_eq!(decoded, message); } + #[test] + fn review_signal_round_trips() { + let message = CoordinationMessage::ReviewSignal(crate::messages::ReviewSignal { + wiki_id: "frwiki".to_string(), + session: sp42_platform::ReviewSession::open("frwiki", "Exemple", 42, 1_000).snapshot(), + }); + + let bytes = encode_message(&message).expect("encoding should succeed"); + let decoded = decode_message(&bytes).expect("decoding should succeed"); + + assert_eq!(decoded, message); + } + fn action_strategy() -> impl Strategy { prop_oneof![ Just(Action::Rollback), diff --git a/crates/sp42-coordination/src/lib.rs b/crates/sp42-coordination/src/lib.rs index 1277e90c..8e541fd4 100644 --- a/crates/sp42-coordination/src/lib.rs +++ b/crates/sp42-coordination/src/lib.rs @@ -22,7 +22,7 @@ pub use codec::{decode_message, encode_message}; pub use errors::{CodecError, CoordinationError}; pub use messages::{ ActionBroadcast, CoordinationMessage, CoordinationRoomSummary, CoordinationSnapshot, EditClaim, - FlaggedEdit, PresenceHeartbeat, RaceResolution, ScoreDelta, + FlaggedEdit, PresenceHeartbeat, RaceResolution, ReviewSignal, ScoreDelta, }; pub use runtime::{CoordinationRuntime, CoordinationRuntimeStatus}; pub use state::{CoordinationState, CoordinationStateSummary}; diff --git a/crates/sp42-coordination/src/messages.rs b/crates/sp42-coordination/src/messages.rs index 099eadb9..dbdb30ee 100644 --- a/crates/sp42-coordination/src/messages.rs +++ b/crates/sp42-coordination/src/messages.rs @@ -1,7 +1,7 @@ //! Coordination protocol payloads and room summary contracts. use serde::{Deserialize, Serialize}; -use sp42_platform::Action; +use sp42_platform::{Action, ReviewSessionSnapshot}; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum CoordinationMessage { @@ -11,6 +11,21 @@ pub enum CoordinationMessage { PresenceHeartbeat(PresenceHeartbeat), FlaggedEdit(FlaggedEdit), RaceResolution(RaceResolution), + ReviewSignal(ReviewSignal), +} + +/// Server-originated hint that a review session changed (PRD-0017, +/// ADR-0018 §8): a live re-fetch cue for operator panels watching the +/// wiki's room. Advisory by design — the review-session store behind the +/// gated `/dev/review` routes stays the source of truth, the reducer +/// relays this kind without folding any room state, and a spoofed signal +/// can only cause a harmless re-fetch. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewSignal { + pub wiki_id: String, + /// Session summary so a panel can badge (status, pending prompts, + /// findings count) without a round-trip. Never authoritative. + pub session: ReviewSessionSnapshot, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/sp42-coordination/src/state.rs b/crates/sp42-coordination/src/state.rs index b1d48ed4..a8f8633e 100644 --- a/crates/sp42-coordination/src/state.rs +++ b/crates/sp42-coordination/src/state.rs @@ -136,6 +136,13 @@ impl CoordinationState { self.recent_actions.drain(0..overflow); } } + CoordinationMessage::ReviewSignal(signal) => { + // Deliberately stateless: the signal is a live re-fetch hint + // for panels, and the review-session store behind the gated + // review routes stays the source of truth (ADR-0018 §8). + // Folding it here would create a second, spoofable copy. + drop(signal); + } } true @@ -202,6 +209,7 @@ fn message_wiki_id(message: &CoordinationMessage) -> &str { CoordinationMessage::PresenceHeartbeat(heartbeat) => &heartbeat.wiki_id, CoordinationMessage::FlaggedEdit(flagged) => &flagged.wiki_id, CoordinationMessage::RaceResolution(resolution) => &resolution.wiki_id, + CoordinationMessage::ReviewSignal(signal) => &signal.wiki_id, } } @@ -209,12 +217,34 @@ fn message_wiki_id(message: &CoordinationMessage) -> &str { mod tests { use crate::messages::{ ActionBroadcast, CoordinationMessage, EditClaim, FlaggedEdit, PresenceHeartbeat, - RaceResolution, ScoreDelta, + RaceResolution, ReviewSignal, ScoreDelta, }; use sp42_platform::Action; use super::CoordinationState; + fn review_signal(wiki_id: &str) -> ReviewSignal { + ReviewSignal { + wiki_id: wiki_id.to_string(), + session: sp42_platform::ReviewSession::open(wiki_id, "Exemple", 42, 1_000).snapshot(), + } + } + + #[test] + fn review_signal_relays_without_folding_room_state() { + let mut state = CoordinationState::new("frwiki"); + let before = state.summary(); + + // Same wiki: accepted (so the relay fans it out to panels)... + assert!(state.apply(CoordinationMessage::ReviewSignal(review_signal("frwiki")))); + // ...but deliberately stateless — the review-session store behind + // the gated review routes is the source of truth (ADR-0018 §8). + assert_eq!(state.summary(), before); + + // Other wiki: rejected like every other kind. + assert!(!state.apply(CoordinationMessage::ReviewSignal(review_signal("enwiki")))); + } + #[test] fn ignores_messages_for_other_wikis() { let mut state = CoordinationState::new("frwiki"); diff --git a/crates/sp42-devtools/src/preview.rs b/crates/sp42-devtools/src/preview.rs index 0b564ea2..fa4254af 100644 --- a/crates/sp42-devtools/src/preview.rs +++ b/crates/sp42-devtools/src/preview.rs @@ -211,6 +211,7 @@ pub fn dev_coordination_message_label(message: &CoordinationMessage) -> &'static CoordinationMessage::PresenceHeartbeat(_) => "PresenceHeartbeat", CoordinationMessage::FlaggedEdit(_) => "FlaggedEdit", CoordinationMessage::RaceResolution(_) => "RaceResolution", + CoordinationMessage::ReviewSignal(_) => "ReviewSignal", } } diff --git a/crates/sp42-platform/src/lib.rs b/crates/sp42-platform/src/lib.rs index efbdca0d..bc93a355 100644 --- a/crates/sp42-platform/src/lib.rs +++ b/crates/sp42-platform/src/lib.rs @@ -48,6 +48,7 @@ pub mod origin; pub mod priority_queue; pub mod public_documents; pub mod queue_builder; +pub mod review_session; pub mod review_workbench; pub mod routes; pub mod score_tier; @@ -127,6 +128,16 @@ pub use public_documents::{ pub use queue_builder::{ build_ranked_queue, build_ranked_queue_with_contexts, build_ranked_queue_with_policy, }; +pub use review_session::{ + FindingsRevisionMismatch, REVIEW_SESSION_CONTRACT_VERSION, ReviewAckResponse, ReviewAnchor, + ReviewBlockOutline, ReviewChatEntry, ReviewChatRole, ReviewEndRequest, ReviewEndedBy, + ReviewFeedbackTake, ReviewFindingMarker, ReviewFindingsRequest, ReviewFindingsResponse, + ReviewOpenRequest, ReviewOpenResponse, ReviewPollRequest, ReviewPollResponse, ReviewPollStatus, + ReviewPrompt, ReviewPromptKind, ReviewQueueRequest, ReviewQueueResponse, ReviewReplyRequest, + ReviewSession, ReviewSessionSnapshot, ReviewSessionStatus, ReviewSessionsResponse, + SessionEnded, annotate_outline, build_article_outline, open_next_step, poll_next_step, + poll_status, truncate_outline_text, +}; pub use review_workbench::{ PreparedRequestPreview, ReviewWorkbench, build_review_workbench, build_session_action_execution_requests, @@ -192,4 +203,5 @@ pub use wikitext_editor::{ ScriptedEditorInvocation, ScriptedWikitextEditor, ScriptedWikitextNode, WikitextEditOutcome, WikitextEditRefusal, WikitextEditor, WikitextEditorError, WikitextNodeDescriptor, WikitextNodeKind, WikitextNodeLocator, WikitextPageRef, normalize_anchor_text, + normalize_title_spacing, }; diff --git a/crates/sp42-platform/src/review_session.rs b/crates/sp42-platform/src/review_session.rs new file mode 100644 index 00000000..70698c9c --- /dev/null +++ b/crates/sp42-platform/src/review_session.rs @@ -0,0 +1,1095 @@ +//! Interactive review-session contracts (PRD-0017). +//! +//! An agent opens a review session on a wiki page, the operator queues +//! anchored feedback prompts against the rendered article, and the agent +//! polls the session to collect that feedback. The loop is ported from the +//! local-first artifact-review pattern popularized by `lavish-axi` +//! (), re-keyed from local file +//! paths to `(wiki_id, title)` targets. +//! +//! This module is the pure core: session state transitions, the +//! drain-before-ended feedback semantics, the operator-ended reopen gate, +//! and the `next_step` guidance strings. The imperative edges — route glue, +//! waiting, notification — live in `sp42-server`; the agent loop lives in +//! `sp42-cli`. + +use serde::{Deserialize, Serialize}; + +use crate::wikitext_editor::{BlockKind, ParsoidBlock}; + +/// Version tag embedded in every review-session response so agents can +/// detect contract drift (Constitution Art. 9.1). +pub const REVIEW_SESSION_CONTRACT_VERSION: u32 = 1; + +/// Outline text is truncated to this many characters so a full-article +/// outline stays a compact, token-friendly snapshot. +pub const REVIEW_OUTLINE_TEXT_LIMIT: usize = 160; + +/// Who closed a review session. Mirrors the `ended_by` etiquette of the +/// ported loop: an operator-ended session refuses a plain reopen, while an +/// agent-ended session may be reopened freely. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewEndedBy { + Operator, + Agent, +} + +/// Lifecycle of a review session. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewSessionStatus { + /// Open with no undelivered feedback. + Open, + /// Open with queued prompts awaiting an agent poll. + Feedback, + /// Closed; `ended_by` records who closed it. + Ended, +} + +/// What a queued prompt is anchored to. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewPromptKind { + /// Anchored to one article block (paragraph, list item, table cell). + Block, + /// Anchored to selected text inside a block. + Text, + /// Free-form message with no article anchor. + Message, +} + +/// Anchor tying a prompt to article structure. Block ordinals and cite ids +/// come from the Parsoid block decomposition, so anchors survive rendering +/// differences that CSS selectors would not. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewAnchor { + /// Document-order index of the anchored block. + pub block_ordinal: usize, + /// Stable cite id (e.g. `"cite_ref-smith_3-0"`) when the prompt targets + /// a specific reference in the block. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ref_id: Option, + /// Verbatim selected text when the prompt targets a text range. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub selected_text: Option, +} + +/// One verification finding projected onto review-session coordinates — the +/// overlay marker that lets a review surface show a page verification +/// report's problems *in the context of the article* instead of as detached +/// report text. Markers are citation-agnostic here: the citation domain +/// projects its report into this shape at the edges (`ref_id` is the join +/// key onto outline blocks), so this crate never depends on report types. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewFindingMarker { + /// The originating ref's marker id (e.g. `cite_ref-smith_3-0`) — the + /// same anchor coordinate prompts use. + pub ref_id: String, + /// Verdict wire label (e.g. `"unsupported"`, `"source_unavailable"`). + pub verdict: String, + /// The judged claim sentence, truncated like outline text. + pub claim: String, + /// Optional short qualifier — a grounding caveat or unavailable reason. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub detail: Option, +} + +/// Refusal reason when attached findings were produced against a different +/// revision than the session is pinned to — a stale overlay would point at +/// blocks that may no longer exist. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct FindingsRevisionMismatch { + /// The revision the session is pinned to. + pub session_rev: u64, +} + +/// One operator feedback item queued for the agent. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewPrompt { + pub kind: ReviewPromptKind, + /// The operator's instruction or question. + pub prompt: String, + /// Article anchor; `None` for [`ReviewPromptKind::Message`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub anchor: Option, +} + +/// Who wrote a chat entry. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewChatRole { + Operator, + Agent, +} + +/// One line of the session's operator/agent conversation. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewChatEntry { + pub role: ReviewChatRole, + pub text: String, + pub at_ms: i64, +} + +/// Pure state of one review session. The server wraps this in its shared +/// store; every transition is a method here so the semantics stay testable +/// without I/O. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewSession { + pub wiki_id: String, + /// Namespace-qualified page title (spaces, not underscores). + pub title: String, + /// Revision the session was opened against (`0` = latest at open time). + pub rev_id: u64, + pub status: ReviewSessionStatus, + /// Prompts queued since the last successful poll delivery. + pub queued_prompts: Vec, + /// Verification findings attached to this session's pinned revision — + /// the report overlay the open response joins onto the outline. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub findings: Vec, + pub chat: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ended_by: Option, + pub created_at_ms: i64, + pub updated_at_ms: i64, +} + +/// Result of draining a session's queued feedback. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ReviewFeedbackTake { + /// Nothing queued and the session is open. + Waiting, + /// Queued prompts, delivered exactly once. `session_ended` is `true` + /// when this is the final batch of a session that has already closed — + /// queued feedback always delivers before the `Ended` status does. + Feedback { + prompts: Vec, + session_ended: bool, + ended_by: Option, + }, + /// Session closed with nothing left to deliver. + Ended { ended_by: ReviewEndedBy }, +} + +/// Refusal reason when an open request hits the reopen gate. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ReopenRefused; + +/// Refusal reason when prompts are queued against an already-ended session. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SessionEnded { + /// Who closed the session. + pub ended_by: ReviewEndedBy, +} + +impl ReviewSession { + /// A fresh open session on `(wiki_id, title)` at `rev_id`. + #[must_use] + pub fn open(wiki_id: &str, title: &str, rev_id: u64, now_ms: i64) -> Self { + Self { + wiki_id: wiki_id.to_string(), + title: title.to_string(), + rev_id, + status: ReviewSessionStatus::Open, + queued_prompts: Vec::new(), + findings: Vec::new(), + chat: Vec::new(), + ended_by: None, + created_at_ms: now_ms, + updated_at_ms: now_ms, + } + } + + /// Canonical store key for a target. The pair is the identity — the + /// analog of the ported loop's canonical-file-path session key. The + /// title component gets the wiki-independent spacing normalization + /// (underscores are spaces, whitespace runs collapse), so every + /// spelling of one page keys one session even for callers that bypass + /// the CLI's target parsing. First-letter case is not folded — case + /// rules are wiki-dependent, and merging two distinct pages would be + /// worse than splitting one. + #[must_use] + pub fn canonical_key(wiki_id: &str, title: &str) -> String { + format!( + "{wiki_id}\u{1f}{}", + crate::wikitext_editor::normalize_title_spacing(title) + ) + } + + /// Gate a reopen attempt: a session the *operator* ended refuses a plain + /// reopen so the agent does not reopen the review surface uninvited; an + /// agent-ended (or still-open) session resumes freely. + /// + /// # Errors + /// + /// [`ReopenRefused`] when the operator ended the session and the caller + /// did not explicitly request a reopen. + pub fn gate_reopen(&self, reopen_requested: bool) -> Result<(), ReopenRefused> { + if self.ended_by == Some(ReviewEndedBy::Operator) && !reopen_requested { + return Err(ReopenRefused); + } + Ok(()) + } + + /// Resume an ended session as open again (after [`Self::gate_reopen`]). + /// Re-pinning to a *different* revision drops the findings overlay: the + /// markers describe the old revision's blocks. + pub fn resume(&mut self, rev_id: u64, now_ms: i64) { + self.status = if self.queued_prompts.is_empty() { + ReviewSessionStatus::Open + } else { + ReviewSessionStatus::Feedback + }; + self.ended_by = None; + if self.rev_id != rev_id { + self.findings.clear(); + } + self.rev_id = rev_id; + self.updated_at_ms = now_ms; + } + + /// Replace the session's findings overlay with one report's markers. + /// Findings do not accumulate across attaches — each attach describes + /// one verification run of the pinned revision. + /// + /// # Errors + /// + /// [`FindingsRevisionMismatch`] when `report_rev_id` differs from the + /// session's pinned revision — a stale overlay must not annotate blocks + /// it was not produced against. + pub fn attach_findings( + &mut self, + report_rev_id: u64, + findings: Vec, + now_ms: i64, + ) -> Result { + if report_rev_id != self.rev_id { + return Err(FindingsRevisionMismatch { + session_rev: self.rev_id, + }); + } + let attached = findings.len(); + self.findings = findings; + self.updated_at_ms = now_ms; + Ok(attached) + } + + /// Queue operator prompts; optionally close the session in the same + /// action ("send & end"). Queued prompts survive the end and deliver on + /// the next poll. + /// + /// # Errors + /// + /// [`SessionEnded`] when the session is already closed: queueing then + /// would silently flip an ended session back to `Feedback` while + /// `ended_by` still says who closed it, so the next poll would deliver + /// with `session_ended: false` and reopen the session around the reopen + /// gate. An ended session takes new prompts only after an explicit open. + pub fn queue_prompts( + &mut self, + prompts: Vec, + end_session: bool, + now_ms: i64, + ) -> Result<(), SessionEnded> { + if self.status == ReviewSessionStatus::Ended { + return Err(SessionEnded { + // An ended session always records who ended it; fall back to + // the operator (the conservative gate) for hand-built state. + ended_by: self.ended_by.unwrap_or(ReviewEndedBy::Operator), + }); + } + self.queued_prompts.extend(prompts); + if end_session { + self.status = ReviewSessionStatus::Ended; + self.ended_by = Some(ReviewEndedBy::Operator); + } else if !self.queued_prompts.is_empty() { + self.status = ReviewSessionStatus::Feedback; + } + self.updated_at_ms = now_ms; + Ok(()) + } + + /// Drain queued feedback with deliver-before-ended semantics: prompts + /// queued before an end still deliver first (marked `session_ended`); + /// only a later poll reports `Ended`. + pub fn take_feedback(&mut self, now_ms: i64) -> ReviewFeedbackTake { + if !self.queued_prompts.is_empty() { + let prompts = std::mem::take(&mut self.queued_prompts); + let session_ended = self.status == ReviewSessionStatus::Ended; + if !session_ended { + self.status = ReviewSessionStatus::Open; + } + self.updated_at_ms = now_ms; + return ReviewFeedbackTake::Feedback { + prompts, + session_ended, + ended_by: self.ended_by, + }; + } + match self.ended_by { + Some(ended_by) if self.status == ReviewSessionStatus::Ended => { + ReviewFeedbackTake::Ended { ended_by } + } + _ => ReviewFeedbackTake::Waiting, + } + } + + /// Append an agent chat reply (shown on the operator surface). + pub fn agent_reply(&mut self, text: &str, now_ms: i64) { + self.chat.push(ReviewChatEntry { + role: ReviewChatRole::Agent, + text: text.to_string(), + at_ms: now_ms, + }); + self.updated_at_ms = now_ms; + } + + /// Close the session, recording who closed it. Ending an already-ended + /// session is a no-op: the first closer keeps the attribution, so an + /// agent's cleanup `end` after an operator's send-and-end cannot + /// downgrade the operator-ended reopen gate to a freely-reopenable + /// agent end. + pub fn end(&mut self, by: ReviewEndedBy, now_ms: i64) { + if self.status == ReviewSessionStatus::Ended { + return; + } + self.status = ReviewSessionStatus::Ended; + self.ended_by = Some(by); + self.updated_at_ms = now_ms; + } + + /// Wire-facing snapshot of this session. + #[must_use] + pub fn snapshot(&self) -> ReviewSessionSnapshot { + ReviewSessionSnapshot { + wiki_id: self.wiki_id.clone(), + title: self.title.clone(), + rev_id: self.rev_id, + status: self.status, + pending_prompts: self.queued_prompts.len(), + findings: self.findings.len(), + ended_by: self.ended_by, + updated_at_ms: self.updated_at_ms, + } + } +} + +/// Wire-facing summary of one session for listings and command output. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewSessionSnapshot { + pub wiki_id: String, + pub title: String, + pub rev_id: u64, + pub status: ReviewSessionStatus, + pub pending_prompts: usize, + /// Attached verification-finding markers (the report overlay). + #[serde(default)] + pub findings: usize, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ended_by: Option, + pub updated_at_ms: i64, +} + +/// One block of the article outline — the compact structure snapshot an +/// agent uses to resolve prompt anchors (the analog of the ported loop's +/// DOM snapshot, built from Parsoid blocks instead of live DOM). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewBlockOutline { + pub block_ordinal: usize, + pub kind: String, + /// Block text truncated to [`REVIEW_OUTLINE_TEXT_LIMIT`] characters. + pub text: String, + /// Stable cite ids of the block's inline references, in order. + pub ref_ids: Vec, + /// Verification findings anchored to this block (the report overlay), + /// joined by `ref_id`. Empty when no report is attached. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub findings: Vec, +} + +/// Build the outline for a decomposed page. +#[must_use] +pub fn build_article_outline(blocks: &[ParsoidBlock]) -> Vec { + blocks + .iter() + .map(|block| ReviewBlockOutline { + block_ordinal: block.block_ordinal, + kind: block_kind_label(block.block_kind).to_string(), + text: truncate_chars(&block.text, REVIEW_OUTLINE_TEXT_LIMIT), + ref_ids: block + .refs + .iter() + .map(|block_ref| block_ref.ref_id.clone()) + .collect(), + findings: Vec::new(), + }) + .collect() +} + +/// Join finding markers onto outline blocks by `ref_id` — the projection +/// that turns the verification report into an in-article overlay. Markers +/// whose `ref_id` matches no block are returned, not dropped: the surface +/// must be able to say "these findings could not be placed" rather than +/// silently under-report. +#[must_use] +pub fn annotate_outline( + mut outline: Vec, + findings: &[ReviewFindingMarker], +) -> (Vec, Vec) { + let mut unanchored = Vec::new(); + for marker in findings { + match outline + .iter_mut() + .find(|block| block.ref_ids.iter().any(|ref_id| ref_id == &marker.ref_id)) + { + Some(block) => block.findings.push(marker.clone()), + None => unanchored.push(marker.clone()), + } + } + (outline, unanchored) +} + +/// Truncate text to the outline display limit — shared by outline blocks +/// and by edge crates projecting report claims into [`ReviewFindingMarker`]s. +#[must_use] +pub fn truncate_outline_text(text: &str) -> String { + truncate_chars(text, REVIEW_OUTLINE_TEXT_LIMIT) +} + +const fn block_kind_label(kind: BlockKind) -> &'static str { + match kind { + BlockKind::Paragraph => "paragraph", + BlockKind::ListItem => "list-item", + BlockKind::TableCell => "table-cell", + BlockKind::Other => "other", + } +} + +/// Truncate on a character boundary, appending an ellipsis when shortened. +fn truncate_chars(text: &str, limit: usize) -> String { + if text.chars().count() <= limit { + return text.to_string(); + } + let mut shortened: String = text.chars().take(limit).collect(); + shortened.push('…'); + shortened +} + +/// Open (or resume) a review session on a page target. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewOpenRequest { + pub wiki_id: String, + /// Bare title or pasted wiki URL; the server unwraps URLs the same way + /// verify-page does. + pub target: String, + /// Pinned revision; `0` means latest. + #[serde(default)] + pub rev_id: u64, + /// Required to resume a session the operator explicitly ended. + #[serde(default)] + pub reopen: bool, +} + +/// Open/resume response: session snapshot plus the article outline. When a +/// verification report is attached, outline blocks carry their findings and +/// markers that matched no block surface in `unanchored_findings`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewOpenResponse { + pub contract_version: u32, + pub session: ReviewSessionSnapshot, + pub outline: Vec, + /// Attached findings whose `ref_id` matched no outline block. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub unanchored_findings: Vec, + /// The session's operator/agent conversation so far — the surface the + /// agent's `--agent-reply` summaries land on; without it a stored reply + /// would be write-only. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub chat: Vec, + pub next_step: String, +} + +/// Agent attaches a verification report's findings to the session (the +/// report becomes an in-article overlay for the operator surface). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewFindingsRequest { + pub wiki_id: String, + pub title: String, + /// Revision the findings were produced against; must match the + /// session's pinned revision. + #[serde(default)] + pub rev_id: u64, + pub findings: Vec, +} + +/// Findings-attach acknowledgement. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewFindingsResponse { + pub contract_version: u32, + pub session: ReviewSessionSnapshot, + pub attached: usize, +} + +/// Operator queues prompts (the browser surface's send action). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewQueueRequest { + pub wiki_id: String, + pub title: String, + pub prompts: Vec, + /// "Send & end": queue and close in one action. + #[serde(default)] + pub end_session: bool, +} + +/// Queue acknowledgement. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewQueueResponse { + pub contract_version: u32, + pub session: ReviewSessionSnapshot, + pub queued: usize, +} + +/// Agent polls for feedback, optionally bounding the server-side wait. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewPollRequest { + pub wiki_id: String, + pub title: String, + /// Server-side wait bound in milliseconds. Omitted (`None`) means the + /// server's default wait; an explicit `0` returns immediately (a + /// nonblocking status check). The server clamps this to its own maximum. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub wait_ms: Option, +} + +/// Poll outcome status on the wire. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReviewPollStatus { + Waiting, + Feedback, + Ended, + Missing, +} + +/// Poll response. `next_step` carries the loop etiquette so agents do not +/// have to invent it: keep polling on `waiting`, apply feedback then poll +/// again with a reply on `feedback`, and stop (without reopening uninvited +/// when the operator ended) on `ended`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewPollResponse { + pub contract_version: u32, + pub status: ReviewPollStatus, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub prompts: Vec, + #[serde(default)] + pub session_ended: bool, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ended_by: Option, + pub next_step: String, +} + +/// Agent chat reply shown on the operator surface. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewReplyRequest { + pub wiki_id: String, + pub title: String, + pub text: String, +} + +/// Agent-initiated session end. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewEndRequest { + pub wiki_id: String, + pub title: String, +} + +/// Generic acknowledgement for reply/end. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewAckResponse { + pub contract_version: u32, + pub session: ReviewSessionSnapshot, +} + +/// Session listing. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ReviewSessionsResponse { + pub contract_version: u32, + pub sessions: Vec, +} + +/// `next_step` for a fresh or resumed open. Mentions the findings overlay +/// when one is attached so the agent tells the operator what to look at. +#[must_use] +pub fn open_next_step(title: &str, findings: usize) -> String { + let overlay = if findings == 0 { + String::new() + } else { + format!(" The outline carries {findings} verification finding(s) anchored to blocks.") + }; + format!( + "Session open on \"{title}\".{overlay} Ask the operator to review it in the SP42 browser \ + shell, then run `sp42-cli review poll` and wait for their feedback before responding \ + further." + ) +} + +/// `next_step` for a poll outcome. +#[must_use] +pub fn poll_next_step(take: &ReviewFeedbackTake, title: &str) -> String { + match take { + ReviewFeedbackTake::Waiting => format!( + "No feedback on \"{title}\" yet. Poll again (the wait is safe to re-run; queued \ + feedback is never lost)." + ), + ReviewFeedbackTake::Feedback { + session_ended: false, + .. + } => format!( + "Apply the operator's feedback on \"{title}\", then poll again with an agent reply \ + summarizing what changed." + ), + ReviewFeedbackTake::Feedback { + session_ended: true, + .. + } => format!( + "Final feedback batch: the operator ended the review of \"{title}\". Apply the \ + feedback and deliver remaining updates in chat; do not reopen unless asked." + ), + ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Operator, + } => format!( + "The operator ended the review of \"{title}\". Stop polling and do not reopen \ + uninvited; deliver any remaining updates in chat." + ), + ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Agent, + } => format!( + "Review of \"{title}\" was ended by the agent. Stop polling; open a new session if \ + further review is needed." + ), + } +} + +/// The wire status for a drained take. +#[must_use] +pub fn poll_status(take: &ReviewFeedbackTake) -> ReviewPollStatus { + match take { + ReviewFeedbackTake::Waiting => ReviewPollStatus::Waiting, + ReviewFeedbackTake::Feedback { .. } => ReviewPollStatus::Feedback, + ReviewFeedbackTake::Ended { .. } => ReviewPollStatus::Ended, + } +} + +#[cfg(test)] +mod tests { + use super::{ + REVIEW_OUTLINE_TEXT_LIMIT, ReopenRefused, ReviewEndedBy, ReviewFeedbackTake, ReviewPrompt, + ReviewPromptKind, ReviewSession, ReviewSessionStatus, annotate_outline, + build_article_outline, open_next_step, poll_next_step, poll_status, + }; + use crate::wikitext_editor::{BlockKind, BlockRef, ParsoidBlock}; + + fn message_prompt(text: &str) -> ReviewPrompt { + ReviewPrompt { + kind: ReviewPromptKind::Message, + prompt: text.to_string(), + anchor: None, + } + } + + #[test] + fn queueing_prompts_moves_the_session_to_feedback() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + + session + .queue_prompts(vec![message_prompt("tighten the lede")], false, 2_000) + .expect("open session should accept prompts"); + + assert_eq!(session.status, ReviewSessionStatus::Feedback); + assert_eq!(session.queued_prompts.len(), 1); + assert_eq!(session.updated_at_ms, 2_000); + } + + #[test] + fn take_feedback_drains_once_and_reopens() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .queue_prompts(vec![message_prompt("first")], false, 2_000) + .expect("open session should accept prompts"); + + let take = session.take_feedback(3_000); + let ReviewFeedbackTake::Feedback { + prompts, + session_ended, + .. + } = take + else { + panic!("expected feedback"); + }; + assert_eq!(prompts.len(), 1); + assert!(!session_ended); + assert_eq!(session.status, ReviewSessionStatus::Open); + assert_eq!(session.take_feedback(4_000), ReviewFeedbackTake::Waiting); + } + + #[test] + fn feedback_queued_before_an_end_delivers_before_ended() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .queue_prompts(vec![message_prompt("last words")], true, 2_000) + .expect("open session should accept prompts"); + + let take = session.take_feedback(3_000); + let ReviewFeedbackTake::Feedback { + session_ended, + ended_by, + .. + } = take + else { + panic!("expected the final feedback batch"); + }; + assert!(session_ended); + assert_eq!(ended_by, Some(ReviewEndedBy::Operator)); + + assert_eq!( + session.take_feedback(4_000), + ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Operator + } + ); + } + + #[test] + fn operator_end_gates_a_plain_reopen_but_agent_end_does_not() { + let mut operator_ended = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + operator_ended.end(ReviewEndedBy::Operator, 2_000); + assert_eq!(operator_ended.gate_reopen(false), Err(ReopenRefused)); + assert_eq!(operator_ended.gate_reopen(true), Ok(())); + + let mut agent_ended = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + agent_ended.end(ReviewEndedBy::Agent, 2_000); + assert_eq!(agent_ended.gate_reopen(false), Ok(())); + } + + #[test] + fn resume_clears_the_end_and_keeps_queued_feedback() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .queue_prompts(vec![message_prompt("pending")], true, 2_000) + .expect("open session should accept prompts"); + + session.resume(43, 3_000); + + assert_eq!(session.status, ReviewSessionStatus::Feedback); + assert_eq!(session.ended_by, None); + assert_eq!(session.rev_id, 43); + assert_eq!(session.queued_prompts.len(), 1); + } + + #[test] + fn canonical_key_separates_wiki_and_title_unambiguously() { + assert_ne!( + ReviewSession::canonical_key("a", "b:c"), + ReviewSession::canonical_key("a:b", "c") + ); + } + + #[test] + fn canonical_key_collapses_title_spellings_but_not_case() { + // Underscore/space/whitespace spellings of one page → one session. + assert_eq!( + ReviewSession::canonical_key("frwiki", "Grand_Exemple"), + ReviewSession::canonical_key("frwiki", "Grand Exemple") + ); + // Case rules are wiki-dependent; never merge across case. + assert_ne!( + ReviewSession::canonical_key("frwiki", "exemple"), + ReviewSession::canonical_key("frwiki", "Exemple") + ); + } + + #[test] + fn outline_truncates_on_character_boundaries() { + let long_text = "é".repeat(REVIEW_OUTLINE_TEXT_LIMIT + 5); + let blocks = vec![ParsoidBlock { + text: long_text, + refs: vec![BlockRef { + offset: 3, + ref_id: "cite_ref-a_1-0".to_string(), + sources: Vec::new(), + book_sources: Vec::new(), + ref_text: "[1]".to_string(), + named: false, + is_bare_url_ref: false, + short_cite_unresolved: false, + }], + block_kind: BlockKind::Paragraph, + block_ordinal: 7, + }]; + + let outline = build_article_outline(&blocks); + + assert_eq!(outline.len(), 1); + assert_eq!(outline[0].block_ordinal, 7); + assert_eq!(outline[0].kind, "paragraph"); + assert_eq!( + outline[0].text.chars().count(), + REVIEW_OUTLINE_TEXT_LIMIT + 1, + "limit characters plus the ellipsis" + ); + assert!(outline[0].text.ends_with('…')); + assert_eq!(outline[0].ref_ids, vec!["cite_ref-a_1-0".to_string()]); + } + + #[test] + fn outline_passes_short_text_through_untruncated() { + let blocks = vec![ParsoidBlock { + text: "short".to_string(), + refs: Vec::new(), + block_kind: BlockKind::ListItem, + block_ordinal: 0, + }]; + + let outline = build_article_outline(&blocks); + + assert_eq!(outline[0].text, "short"); + assert_eq!(outline[0].kind, "list-item"); + } + + #[test] + fn next_step_matches_the_loop_etiquette() { + let waiting = poll_next_step(&ReviewFeedbackTake::Waiting, "Exemple"); + assert!(waiting.contains("Poll again")); + + let operator_ended = poll_next_step( + &ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Operator, + }, + "Exemple", + ); + assert!(operator_ended.contains("do not reopen")); + + let agent_ended = poll_next_step( + &ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Agent, + }, + "Exemple", + ); + assert!(agent_ended.contains("new session")); + } + + #[test] + fn poll_status_maps_every_take() { + use super::ReviewPollStatus; + assert_eq!( + poll_status(&ReviewFeedbackTake::Waiting), + ReviewPollStatus::Waiting + ); + assert_eq!( + poll_status(&ReviewFeedbackTake::Feedback { + prompts: Vec::new(), + session_ended: false, + ended_by: None, + }), + ReviewPollStatus::Feedback + ); + assert_eq!( + poll_status(&ReviewFeedbackTake::Ended { + ended_by: ReviewEndedBy::Agent + }), + ReviewPollStatus::Ended + ); + } + + #[test] + fn agent_end_does_not_downgrade_an_operator_end() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session.end(ReviewEndedBy::Operator, 2_000); + + // Agent cleanup after the operator already closed the review must + // not flip the attribution and lift the reopen gate. + session.end(ReviewEndedBy::Agent, 3_000); + + assert_eq!(session.ended_by, Some(ReviewEndedBy::Operator)); + assert_eq!(session.gate_reopen(false), Err(ReopenRefused)); + assert_eq!( + session.updated_at_ms, 2_000, + "a no-op end leaves the session untouched" + ); + } + + #[test] + fn queueing_to_an_ended_session_is_refused() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .queue_prompts(vec![message_prompt("last words")], true, 2_000) + .expect("open session should accept the send-and-end"); + + // A late queue (stale panel, plain CLI queue) must not flip the + // ended session back to feedback around the reopen gate. + let refused = session + .queue_prompts(vec![message_prompt("too late")], false, 3_000) + .expect_err("ended session must refuse new prompts"); + assert_eq!(refused.ended_by, ReviewEndedBy::Operator); + assert_eq!(session.status, ReviewSessionStatus::Ended); + assert_eq!( + session.queued_prompts.len(), + 1, + "only the pre-end prompt stays queued" + ); + + // The pre-end feedback still delivers with the end flag intact. + let take = session.take_feedback(4_000); + let ReviewFeedbackTake::Feedback { session_ended, .. } = take else { + panic!("expected the final feedback batch"); + }; + assert!(session_ended); + } + + #[test] + fn attach_findings_replaces_the_overlay_and_counts() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + + let attached = session + .attach_findings(42, vec![marker("cite_ref-a_1-0", "unsupported")], 2_000) + .expect("matching revision should attach"); + assert_eq!(attached, 1); + assert_eq!(session.findings.len(), 1); + assert_eq!(session.updated_at_ms, 2_000); + + // A later attach replaces the overlay wholesale — findings describe + // one report, they do not accumulate across runs. + let attached = session + .attach_findings( + 42, + vec![ + marker("cite_ref-b_2-0", "partial"), + marker("cite_ref-c_3-0", "source_unavailable"), + ], + 3_000, + ) + .expect("matching revision should attach"); + assert_eq!(attached, 2); + assert_eq!(session.findings.len(), 2); + assert_eq!(session.findings[0].ref_id, "cite_ref-b_2-0"); + } + + #[test] + fn attach_findings_refuses_a_revision_mismatch() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + + let refused = session + .attach_findings(41, vec![marker("cite_ref-a_1-0", "unsupported")], 2_000) + .expect_err("stale-revision findings must not overlay the outline"); + assert_eq!(refused.session_rev, 42); + assert!(session.findings.is_empty()); + } + + #[test] + fn resume_to_a_new_revision_drops_stale_findings() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .attach_findings(42, vec![marker("cite_ref-a_1-0", "unsupported")], 2_000) + .expect("attach"); + session.end(ReviewEndedBy::Agent, 3_000); + + session.resume(43, 4_000); + assert!( + session.findings.is_empty(), + "findings for revision 42 must not overlay revision 43" + ); + + let mut same_rev = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + same_rev + .attach_findings(42, vec![marker("cite_ref-a_1-0", "unsupported")], 2_000) + .expect("attach"); + same_rev.resume(42, 3_000); + assert_eq!( + same_rev.findings.len(), + 1, + "same-revision resume keeps the overlay" + ); + } + + #[test] + fn annotate_outline_joins_findings_onto_blocks_by_ref_id() { + let outline = vec![ + outline_block(0, vec!["cite_ref-a_1-0"]), + outline_block(1, vec!["cite_ref-b_2-0", "cite_ref-c_3-0"]), + ]; + let findings = vec![ + marker("cite_ref-c_3-0", "unsupported"), + marker("cite_ref-ghost_9-0", "partial"), + ]; + + let (annotated, unanchored) = annotate_outline(outline, &findings); + + assert!(annotated[0].findings.is_empty()); + assert_eq!(annotated[1].findings.len(), 1); + assert_eq!(annotated[1].findings[0].verdict, "unsupported"); + assert_eq!(unanchored.len(), 1, "unmatched markers surface, never drop"); + assert_eq!(unanchored[0].ref_id, "cite_ref-ghost_9-0"); + } + + #[test] + fn snapshot_counts_attached_findings() { + let mut session = ReviewSession::open("frwiki", "Exemple", 42, 1_000); + session + .attach_findings(42, vec![marker("cite_ref-a_1-0", "unsupported")], 2_000) + .expect("attach"); + assert_eq!(session.snapshot().findings, 1); + } + + #[test] + fn open_next_step_mentions_an_attached_overlay() { + assert!(!open_next_step("Exemple", 0).contains("finding")); + assert!(open_next_step("Exemple", 3).contains("3 verification finding")); + } + + fn marker(ref_id: &str, verdict: &str) -> super::ReviewFindingMarker { + super::ReviewFindingMarker { + ref_id: ref_id.to_string(), + verdict: verdict.to_string(), + claim: "a claim".to_string(), + detail: None, + } + } + + fn outline_block(block_ordinal: usize, ref_ids: Vec<&str>) -> super::ReviewBlockOutline { + super::ReviewBlockOutline { + block_ordinal, + kind: "paragraph".to_string(), + text: "text".to_string(), + ref_ids: ref_ids.into_iter().map(str::to_string).collect(), + findings: Vec::new(), + } + } + + #[test] + fn wire_types_round_trip_with_the_contract_version() { + let response = super::ReviewPollResponse { + contract_version: super::REVIEW_SESSION_CONTRACT_VERSION, + status: super::ReviewPollStatus::Feedback, + prompts: vec![ReviewPrompt { + kind: ReviewPromptKind::Text, + prompt: "check this quote".to_string(), + anchor: Some(super::ReviewAnchor { + block_ordinal: 3, + ref_id: Some("cite_ref-x_2-0".to_string()), + selected_text: Some("the quote".to_string()), + }), + }], + session_ended: false, + ended_by: None, + next_step: "apply".to_string(), + }; + + let json = serde_json::to_string(&response).expect("poll response should serialize"); + let parsed: super::ReviewPollResponse = + serde_json::from_str(&json).expect("poll response should deserialize"); + + assert_eq!(parsed, response); + assert_eq!(parsed.contract_version, 1); + } +} diff --git a/crates/sp42-platform/src/routes.rs b/crates/sp42-platform/src/routes.rs index 162f0c28..a1c6499a 100644 --- a/crates/sp42-platform/src/routes.rs +++ b/crates/sp42-platform/src/routes.rs @@ -99,6 +99,18 @@ pub const DEV_CITATION_VERIFY_PAGE_PATH: &str = "/dev/citation/verify-page"; /// cost is spent. pub const DEV_CITATION_REVERIFY_PATH: &str = "/dev/citation/reverify"; +/// Interactive review-session bridge (PRD-0017): an agent opens a session on +/// a page, the operator queues anchored feedback, and the agent polls for it. +/// All POST routes are session+CSRF gated; the listing is a read-only +/// inspection surface. +pub const DEV_REVIEW_OPEN_PATH: &str = "/dev/review/open"; +pub const DEV_REVIEW_SESSIONS_PATH: &str = "/dev/review/sessions"; +pub const DEV_REVIEW_PROMPTS_PATH: &str = "/dev/review/prompts"; +pub const DEV_REVIEW_POLL_PATH: &str = "/dev/review/poll"; +pub const DEV_REVIEW_FINDINGS_PATH: &str = "/dev/review/findings"; +pub const DEV_REVIEW_REPLY_PATH: &str = "/dev/review/agent-reply"; +pub const DEV_REVIEW_END_PATH: &str = "/dev/review/end"; + /// Header carrying the bridge session's CSRF token on state-changing routes. pub const CSRF_HEADER_NAME: &str = "x-sp42-csrf-token"; @@ -336,6 +348,11 @@ fn operator_dev_endpoints(default_wiki_id: &str) -> Vec { COORDINATION_INSPECTIONS_PATH, "Room-by-room coordination inspection collection.", ), + RouteDefinition::new( + RouteMethod::Get, + DEV_REVIEW_SESSIONS_PATH, + "Interactive review-session inventory and pending-feedback summaries.", + ), ] } diff --git a/crates/sp42-platform/src/wikitext_editor.rs b/crates/sp42-platform/src/wikitext_editor.rs index 85c03742..2351e8b8 100644 --- a/crates/sp42-platform/src/wikitext_editor.rs +++ b/crates/sp42-platform/src/wikitext_editor.rs @@ -52,6 +52,19 @@ pub struct WikitextNodeLocator { pub expected_text: String, } +/// The spacing half of `MediaWiki` title normalization, valid on every +/// wiki: underscores are spaces, and runs of whitespace collapse to one +/// space. First-letter case is deliberately *not* folded — case rules are +/// wiki-dependent (Wiktionary titles are case-sensitive), and conflating +/// two distinct pages would be worse than treating one page as two. +#[must_use] +pub fn normalize_title_spacing(raw: &str) -> String { + raw.replace('_', " ") + .split_whitespace() + .collect::>() + .join(" ") +} + /// The page revision a node-anchored operation grounds on. #[derive(Debug, Clone, PartialEq, Eq)] pub struct WikitextPageRef { diff --git a/crates/sp42-server/src/citation_routes.rs b/crates/sp42-server/src/citation_routes.rs index 1f627bb1..87910cc3 100644 --- a/crates/sp42-server/src/citation_routes.rs +++ b/crates/sp42-server/src/citation_routes.rs @@ -212,7 +212,7 @@ pub(crate) async fn post_bare_url_proposals( /// Resolve a page's current revision id via the wiki action API (a public read). /// Used when a verify-page request leaves `rev_id` at `0` ("latest"). -async fn fetch_latest_revid( +pub(crate) async fn fetch_latest_revid( client: &(impl HttpClient + ?Sized), config: &sp42_core::WikiConfig, title: &str, diff --git a/crates/sp42-server/src/main.rs b/crates/sp42-server/src/main.rs index 876ae7dd..c0f7dafb 100644 --- a/crates/sp42-server/src/main.rs +++ b/crates/sp42-server/src/main.rs @@ -11,6 +11,7 @@ mod local_env; mod oauth_runtime; mod operator_live; mod parsoid_editor; +mod review_routes; mod revision_artifacts; mod routes; pub(crate) mod runtime_adapters; @@ -322,6 +323,7 @@ async fn main() -> Result<(), std::io::Error> { capability_targets: CapabilityProbeTargets::default(), clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: review_routes::new_review_session_store(), deployment, wiki_registry, wikitext_editor: Arc::new(parsoid_editor::ParsoidWikitextEditor::new()), diff --git a/crates/sp42-server/src/review_routes.rs b/crates/sp42-server/src/review_routes.rs new file mode 100644 index 00000000..ade9b129 --- /dev/null +++ b/crates/sp42-server/src/review_routes.rs @@ -0,0 +1,627 @@ +//! Interactive review-session bridge routes (PRD-0017). +//! +//! FCIS: every session transition (queue, drain-before-ended, reopen gate, +//! end etiquette) is a pure `sp42_core::ReviewSession` method; this module +//! owns the imperative edges — the shared store, the bounded poll wait, the +//! wake-up notification, and the route glue. All POST routes are +//! session+CSRF gated (ADR-0002): the agent side rides the CLI's bridge +//! bootstrap, the operator side rides the browser session cookie. + +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; + +use axum::{ + Json, + extract::State, + http::{HeaderMap, StatusCode}, + response::IntoResponse, +}; +use sp42_coordination::{CoordinationMessage, ReviewSignal}; +use sp42_core::{ + REVIEW_SESSION_CONTRACT_VERSION, ReviewAckResponse, ReviewBlockOutline, ReviewEndRequest, + ReviewEndedBy, ReviewFeedbackTake, ReviewFindingsRequest, ReviewFindingsResponse, + ReviewOpenRequest, ReviewOpenResponse, ReviewPollRequest, ReviewPollResponse, ReviewPollStatus, + ReviewQueueRequest, ReviewQueueResponse, ReviewSession, ReviewSessionSnapshot, + ReviewSessionsResponse, WikitextPageRef, annotate_outline, build_article_outline, + open_next_step, poll_next_step, poll_status, +}; +use tokio::sync::{Notify, RwLock}; +use tracing::{info, warn}; + +use crate::coordination::CoordinationEnvelope; + +use crate::citation_routes::fetch_latest_revid; +use crate::config_for_state_wiki; +use crate::http_errors::unauthorized_error; +use crate::runtime_adapters::BearerHttpClient; +use crate::session_runtime::{current_session_snapshot, validate_csrf_header}; +use crate::state::AppState; + +/// Default server-side wait when a poll request does not bound it. +pub(crate) const POLL_WAIT_DEFAULT_MS: u64 = 25_000; + +/// Upper clamp on the server-side poll wait: long enough that an agent loop +/// is quiet, short enough to stay under common client/proxy request +/// timeouts. The CLI re-arms between waits, so the loop is unbounded even +/// though each request is not. +pub(crate) const POLL_WAIT_MAX_MS: u64 = 55_000; + +/// One stored session plus its poll wake-up channel. `notify_one` stores a +/// permit when no poll is waiting, so feedback queued between an empty drain +/// and the wait registration still wakes the next `notified().await`. +#[derive(Debug)] +pub(crate) struct ReviewSessionEntry { + pub(crate) session: ReviewSession, + pub(crate) notify: Arc, +} + +pub(crate) type SharedReviewSessions = Arc>>; + +/// Fresh empty review-session store for state construction. +pub(crate) fn new_review_session_store() -> SharedReviewSessions { + Arc::new(RwLock::new(HashMap::new())) +} + +type RouteError = (StatusCode, Json); + +/// Sender id for server-originated room messages. Client ids are handed out +/// from 1, so 0 is never echo-suppressed by the relay. +pub(crate) const SERVER_SENDER_ID: u64 = 0; + +/// Publish the live re-fetch hint for a changed review session to the +/// wiki's coordination room (ADR-0018 §8). Best-effort by design: a room no +/// operator has joined drops the signal, and panels recover through the +/// review routes — the signal is a doorbell, never the data. +async fn publish_review_signal(state: &AppState, session: &sp42_core::ReviewSessionSnapshot) { + let message = CoordinationMessage::ReviewSignal(ReviewSignal { + wiki_id: session.wiki_id.clone(), + session: session.clone(), + }); + match sp42_coordination::encode_message(&message) { + Ok(payload) => { + let _subscribers = state + .coordination + .publish( + &session.wiki_id, + CoordinationEnvelope { + sender_id: SERVER_SENDER_ID, + payload, + }, + ) + .await; + } + Err(error) => warn!(%error, "review signal failed to encode"), + } +} + +/// Require an authenticated bridge session with a valid CSRF header — +/// the shared gate for every review POST route. +async fn require_review_session( + state: &AppState, + headers: &HeaderMap, +) -> Result { + let Some(session) = current_session_snapshot(state, headers, true).await else { + return Err(unauthorized_error( + "No authenticated bridge session is active.", + )); + }; + validate_csrf_header(headers, &session)?; + Ok(session) +} + +fn missing_session_error(wiki_id: &str, title: &str) -> RouteError { + ( + StatusCode::NOT_FOUND, + Json(serde_json::json!({ + "error": format!("no review session is open for {title} on {wiki_id}"), + "code": "review-session-not-found", + })), + ) +} + +/// Resolve the open target: unwrap pasted URLs, pin `rev_id == 0` to the +/// current revision (so the outline and the session record agree on what +/// was reviewed), and decompose the page into outline blocks. +async fn resolve_open_target( + state: &AppState, + session: &crate::state::SessionSnapshot, + payload: &ReviewOpenRequest, +) -> Result<(String, u64, Vec), RouteError> { + let config = config_for_state_wiki(state, &payload.wiki_id)?; + let target = sp42_core::parse_page_target(&payload.target); + let mut rev_id = if payload.rev_id == 0 { + target.rev_id + } else { + payload.rev_id + }; + if rev_id == 0 { + let wiki_client = + BearerHttpClient::new(state.http_client.clone(), session.access_token.clone()); + rev_id = fetch_latest_revid(&wiki_client, &config, &target.title) + .await + .map_err(|error| crate::action_routes::action_error_response(&error))?; + } + let page_ref = WikitextPageRef { + title: target.title.clone(), + rev_id, + }; + let blocks = state + .wikitext_editor + .extract_blocks(&config, &page_ref) + .await + .map_err(|error| { + crate::action_routes::action_error_response( + &crate::action_routes::action_error_from_editor(&error), + ) + })?; + Ok((target.title, rev_id, build_article_outline(&blocks))) +} + +/// The 409 etiquette response when a plain open hits a session the operator +/// explicitly ended. +fn reopen_gate_error( + session: &ReviewSession, + reopen_requested: bool, + title: &str, +) -> Result<(), RouteError> { + if session.gate_reopen(reopen_requested).is_err() { + return Err(( + StatusCode::CONFLICT, + Json(serde_json::json!({ + "error": format!( + "the operator ended the review of {title}; do not reopen uninvited — \ + pass reopen=true only when the operator asks for further review" + ), + "code": "review-session-operator-ended", + })), + )); + } + Ok(()) +} + +/// Open a new session or resume an existing one, honoring the reopen gate: +/// a session the operator explicitly ended refuses a plain reopen. +fn open_or_resume( + entry: Option<&mut ReviewSessionEntry>, + reopen_requested: bool, + title: &str, + rev_id: u64, + now_ms: i64, +) -> Result, RouteError> { + let Some(entry) = entry else { + return Ok(None); + }; + reopen_gate_error(&entry.session, reopen_requested, title)?; + entry.session.resume(rev_id, now_ms); + Ok(Some(entry.session.snapshot())) +} + +/// `POST /dev/review/open` — open or resume a review session on a page. +/// +/// Session + CSRF gated: the route mutates server state and (for a latest- +/// revision open) reads through the caller's wiki identity. +pub(crate) async fn post_review_open( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + let session = require_review_session(&state, &headers).await?; + + // Check the reopen gate before any remote read: a plain reopen of an + // operator-ended session must get the 409 etiquette response from the + // stored session alone, not depend on (or pay for) revision/Parsoid + // lookups. The key from the locally parsed title matches the one built + // after resolution. Re-gated under the write lock below for the race. + let parsed_title = sp42_core::parse_page_target(&payload.target).title; + { + let store = state.review_sessions.read().await; + let key = ReviewSession::canonical_key(&payload.wiki_id, &parsed_title); + if let Some(entry) = store.get(&key) { + reopen_gate_error(&entry.session, payload.reopen, &parsed_title)?; + } + } + + let (title, rev_id, outline) = resolve_open_target(&state, &session, &payload).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &title); + let now_ms = state.clock.now_ms(); + + let mut store = state.review_sessions.write().await; + let resumed = open_or_resume(store.get_mut(&key), payload.reopen, &title, rev_id, now_ms)?; + let (session, findings, chat) = if let Some(snapshot) = resumed { + let (findings, chat) = store + .get(&key) + .map(|entry| (entry.session.findings.clone(), entry.session.chat.clone())) + .unwrap_or_default(); + (snapshot, findings, chat) + } else { + let session = ReviewSession::open(&payload.wiki_id, &title, rev_id, now_ms); + let snapshot = session.snapshot(); + store.insert( + key, + ReviewSessionEntry { + session, + notify: Arc::new(Notify::new()), + }, + ); + (snapshot, Vec::new(), Vec::new()) + }; + drop(store); + publish_review_signal(&state, &session).await; + // Attached verification findings overlay the outline: each marker joins + // its block by ref_id, so the operator sees report problems in article + // context rather than as detached text. + let (outline, unanchored_findings) = annotate_outline(outline, &findings); + info!( + wiki_id = %session.wiki_id, + title = %session.title, + rev_id = session.rev_id, + findings = findings.len(), + "review session opened" + ); + Ok(Json(ReviewOpenResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + next_step: open_next_step(&session.title, findings.len()), + session, + outline, + unanchored_findings, + chat, + })) +} + +/// `GET /dev/review/sessions` — read-only session inventory. +pub(crate) async fn get_review_sessions(State(state): State) -> impl IntoResponse { + let store = state.review_sessions.read().await; + let mut sessions: Vec = store + .values() + .map(|entry| entry.session.snapshot()) + .collect(); + sessions.sort_by_key(|session| std::cmp::Reverse(session.updated_at_ms)); + Json(ReviewSessionsResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + sessions, + }) +} + +/// `POST /dev/review/prompts` — the operator queues feedback (optionally +/// ending the session in the same action) and wakes any waiting poll. +pub(crate) async fn post_review_prompts( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + require_review_session(&state, &headers).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &payload.title); + let now_ms = state.clock.now_ms(); + + let mut store = state.review_sessions.write().await; + let Some(entry) = store.get_mut(&key) else { + return Err(missing_session_error(&payload.wiki_id, &payload.title)); + }; + let queued = payload.prompts.len(); + entry + .session + .queue_prompts(payload.prompts, payload.end_session, now_ms) + .map_err(|ended| { + let by = match ended.ended_by { + ReviewEndedBy::Operator => "operator", + ReviewEndedBy::Agent => "agent", + }; + ( + StatusCode::CONFLICT, + Json(serde_json::json!({ + "error": format!( + "the review of {} was already ended by the {by}; open the session \ + again before queueing more feedback", + payload.title + ), + "code": "review-session-ended", + })), + ) + })?; + entry.notify.notify_one(); + let snapshot = entry.session.snapshot(); + drop(store); + publish_review_signal(&state, &snapshot).await; + info!( + wiki_id = %payload.wiki_id, + title = %payload.title, + queued, + end_session = payload.end_session, + "review feedback queued" + ); + Ok(Json(ReviewQueueResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + session: snapshot, + queued, + })) +} + +/// `POST /dev/review/findings` — the agent attaches a verification report's +/// finding markers to the session (replace-all; a stale-revision report is +/// refused). The markers overlay the outline on the next open and feed the +/// operator surface; they are not agent feedback, so no poll is woken. +pub(crate) async fn post_review_findings( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + require_review_session(&state, &headers).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &payload.title); + let now_ms = state.clock.now_ms(); + let mut store = state.review_sessions.write().await; + let Some(entry) = store.get_mut(&key) else { + return Err(missing_session_error(&payload.wiki_id, &payload.title)); + }; + let attached = entry + .session + .attach_findings(payload.rev_id, payload.findings, now_ms) + .map_err(|mismatch| { + ( + StatusCode::CONFLICT, + Json(serde_json::json!({ + "error": format!( + "the findings were produced against revision {} but the review \ + session is pinned to revision {}; re-run verification (or reopen \ + the session) on one revision", + payload.rev_id, mismatch.session_rev + ), + "code": "review-findings-revision-mismatch", + })), + ) + })?; + let snapshot = entry.session.snapshot(); + drop(store); + publish_review_signal(&state, &snapshot).await; + info!( + wiki_id = %payload.wiki_id, + title = %payload.title, + attached, + "review findings attached" + ); + Ok(Json(ReviewFindingsResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + session: snapshot, + attached, + })) +} + +/// Drain one session's feedback; `None` when the session does not exist. +async fn drain_feedback( + state: &AppState, + key: &str, +) -> Option<(ReviewFeedbackTake, Arc, String)> { + let mut store = state.review_sessions.write().await; + let entry = store.get_mut(key)?; + let take = entry.session.take_feedback(state.clock.now_ms()); + Some((take, entry.notify.clone(), entry.session.title.clone())) +} + +fn poll_response(take: ReviewFeedbackTake, title: &str) -> ReviewPollResponse { + let status = poll_status(&take); + let next_step = poll_next_step(&take, title); + let (prompts, session_ended, ended_by) = match take { + ReviewFeedbackTake::Waiting => (Vec::new(), false, None), + ReviewFeedbackTake::Feedback { + prompts, + session_ended, + ended_by, + } => (prompts, session_ended, ended_by), + ReviewFeedbackTake::Ended { ended_by } => (Vec::new(), true, Some(ended_by)), + }; + ReviewPollResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + status, + prompts, + session_ended, + ended_by, + next_step, + } +} + +fn missing_poll_response() -> ReviewPollResponse { + ReviewPollResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + status: ReviewPollStatus::Missing, + prompts: Vec::new(), + session_ended: false, + ended_by: None, + next_step: "No review session is open for this page. Run `sp42-cli review open` first." + .to_string(), + } +} + +/// `POST /dev/review/poll` — the agent's bounded feedback wait. +/// +/// Drains immediately when feedback is queued; otherwise waits (clamped to +/// [`POLL_WAIT_MAX_MS`]; an explicit `wait_ms: 0` skips the wait entirely +/// for a nonblocking status check) for a queue/end wake-up and drains once +/// more. The +/// wake-up uses `notify_one` permits, so feedback queued while no poll is +/// waiting is picked up by the next poll without racing. +pub(crate) async fn post_review_poll( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + require_review_session(&state, &headers).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &payload.title); + let Some((take, notify, title)) = drain_feedback(&state, &key).await else { + return Ok(Json(missing_poll_response())); + }; + + // Omitted wait = the server default; an explicit 0 is the contract's + // nonblocking status check and must return the first drain immediately. + let wait_ms = payload + .wait_ms + .map_or(POLL_WAIT_DEFAULT_MS, |ms| ms.min(POLL_WAIT_MAX_MS)); + let take = if take == ReviewFeedbackTake::Waiting && wait_ms > 0 { + let _ = tokio::time::timeout(Duration::from_millis(wait_ms), notify.notified()).await; + match drain_feedback(&state, &key).await { + Some((take, _, _)) => take, + None => return Ok(Json(missing_poll_response())), + } + } else { + take + }; + + if !matches!(take, ReviewFeedbackTake::Waiting) { + info!( + wiki_id = %payload.wiki_id, + title = %title, + status = ?poll_status(&take), + "review poll delivered" + ); + } + // A delivered batch changed the session (pending count, maybe status) — + // ring the room so a panel's badge does not sit on the stale count from + // the queueing signal until some later mutation. + if matches!(take, ReviewFeedbackTake::Feedback { .. }) { + let snapshot = state + .review_sessions + .read() + .await + .get(&key) + .map(|entry| entry.session.snapshot()); + if let Some(snapshot) = snapshot { + publish_review_signal(&state, &snapshot).await; + } + } + Ok(Json(poll_response(take, &title))) +} + +/// `POST /dev/review/agent-reply` — the agent's chat message to the +/// operator surface. +pub(crate) async fn post_review_reply( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + require_review_session(&state, &headers).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &payload.title); + let now_ms = state.clock.now_ms(); + let mut store = state.review_sessions.write().await; + let Some(entry) = store.get_mut(&key) else { + return Err(missing_session_error(&payload.wiki_id, &payload.title)); + }; + entry.session.agent_reply(&payload.text, now_ms); + let snapshot = entry.session.snapshot(); + drop(store); + publish_review_signal(&state, &snapshot).await; + Ok(Json(ReviewAckResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + session: snapshot, + })) +} + +/// `POST /dev/review/end` — agent-initiated end. Unlike an operator end, a +/// plain reopen stays allowed afterwards. +pub(crate) async fn post_review_end( + State(state): State, + headers: HeaderMap, + Json(payload): Json, +) -> Result { + require_review_session(&state, &headers).await?; + let key = ReviewSession::canonical_key(&payload.wiki_id, &payload.title); + let now_ms = state.clock.now_ms(); + let mut store = state.review_sessions.write().await; + 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); + entry.notify.notify_one(); + let snapshot = entry.session.snapshot(); + drop(store); + publish_review_signal(&state, &snapshot).await; + info!( + wiki_id = %payload.wiki_id, + title = %payload.title, + "review session ended by agent" + ); + Ok(Json(ReviewAckResponse { + contract_version: REVIEW_SESSION_CONTRACT_VERSION, + session: snapshot, + })) +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + use std::time::Duration; + + use sp42_core::{ReviewFeedbackTake, ReviewPrompt, ReviewPromptKind, ReviewSession}; + use tokio::sync::Notify; + + use super::{ReviewSessionEntry, new_review_session_store}; + + fn message_prompt(text: &str) -> ReviewPrompt { + ReviewPrompt { + kind: ReviewPromptKind::Message, + prompt: text.to_string(), + anchor: None, + } + } + + #[tokio::test] + async fn queued_feedback_wakes_a_waiting_poll() { + let store = new_review_session_store(); + let key = ReviewSession::canonical_key("frwiki", "Exemple"); + store.write().await.insert( + key.clone(), + ReviewSessionEntry { + session: ReviewSession::open("frwiki", "Exemple", 42, 1_000), + notify: Arc::new(Notify::new()), + }, + ); + + let waiter_store = store.clone(); + let waiter_key = key.clone(); + let waiter = tokio::spawn(async move { + let notify = waiter_store + .read() + .await + .get(&waiter_key) + .expect("session should exist") + .notify + .clone(); + tokio::time::timeout(Duration::from_secs(5), notify.notified()) + .await + .expect("queueing should wake the waiter"); + let mut sessions = waiter_store.write().await; + let entry = sessions + .get_mut(&waiter_key) + .expect("session should still exist"); + entry.session.take_feedback(2_000) + }); + + // Let the waiter register before queueing. + tokio::time::sleep(Duration::from_millis(50)).await; + { + let mut sessions = store.write().await; + let entry = sessions.get_mut(&key).expect("session should exist"); + entry + .session + .queue_prompts(vec![message_prompt("wake up")], false, 1_500) + .expect("open session should accept prompts"); + entry.notify.notify_one(); + } + + let take = waiter.await.expect("waiter should finish"); + let ReviewFeedbackTake::Feedback { prompts, .. } = take else { + panic!("expected delivered feedback"); + }; + assert_eq!(prompts.len(), 1); + assert_eq!(prompts[0].prompt, "wake up"); + } + + #[tokio::test] + async fn a_permit_covers_feedback_queued_before_the_wait_registers() { + let notify = Arc::new(Notify::new()); + // Feedback arrives while no poll is waiting: the permit is stored... + notify.notify_one(); + // ...and the next wait completes immediately instead of blocking. + tokio::time::timeout(Duration::from_millis(100), notify.notified()) + .await + .expect("stored permit should complete the wait immediately"); + } +} diff --git a/crates/sp42-server/src/routes.rs b/crates/sp42-server/src/routes.rs index c0ac2c6f..54e68845 100644 --- a/crates/sp42-server/src/routes.rs +++ b/crates/sp42-server/src/routes.rs @@ -18,6 +18,10 @@ use crate::citation_routes::{ post_bare_url_apply, post_bare_url_proposals, post_citation_reverify, post_verify_page, }; use crate::operator_live::get_live_operator_view; +use crate::review_routes::{ + get_review_sessions, post_review_end, post_review_findings, post_review_open, post_review_poll, + post_review_prompts, post_review_reply, +}; use crate::revision_artifacts::{ get_rendered_hunk_preview, get_revision_content_diff, get_revision_diff, get_revision_media_diff, @@ -85,9 +89,44 @@ fn operator_routes(router: Router) -> Router { let router = operator_api_routes(router); let router = operator_storage_routes(router); let router = dev_bridge_routes(router); + let router = review_session_routes(router); static_asset_routes(router) } +/// Interactive review-session bridge (PRD-0017): agent opens/polls, the +/// operator queues feedback, both through the same session-gated bridge. +fn review_session_routes(router: Router) -> Router { + router + .route( + route_contracts::DEV_REVIEW_OPEN_PATH, + axum::routing::post(post_review_open), + ) + .route( + route_contracts::DEV_REVIEW_SESSIONS_PATH, + get(get_review_sessions), + ) + .route( + route_contracts::DEV_REVIEW_PROMPTS_PATH, + axum::routing::post(post_review_prompts), + ) + .route( + route_contracts::DEV_REVIEW_POLL_PATH, + axum::routing::post(post_review_poll), + ) + .route( + route_contracts::DEV_REVIEW_FINDINGS_PATH, + axum::routing::post(post_review_findings), + ) + .route( + route_contracts::DEV_REVIEW_REPLY_PATH, + axum::routing::post(post_review_reply), + ) + .route( + route_contracts::DEV_REVIEW_END_PATH, + axum::routing::post(post_review_end), + ) +} + fn coordination_routes(router: Router) -> Router { router .route( diff --git a/crates/sp42-server/src/state.rs b/crates/sp42-server/src/state.rs index 90629894..0d8c85e6 100644 --- a/crates/sp42-server/src/state.rs +++ b/crates/sp42-server/src/state.rs @@ -13,6 +13,7 @@ use crate::coordination::CoordinationRegistry; use crate::deployment::DeploymentConfig; use crate::live_queue::IngestionSupervisorSnapshot; use crate::local_env::LocalOAuthConfig; +use crate::review_routes::SharedReviewSessions; use crate::revision_artifacts::{CachedRenderedHunkPreview, CachedRevisionArtifacts}; use crate::wikimedia_capabilities::CapabilityProbeTargets; @@ -38,6 +39,7 @@ pub(crate) struct AppState { pub(crate) capability_targets: CapabilityProbeTargets, pub(crate) clock: Arc, pub(crate) coordination: CoordinationRegistry, + pub(crate) review_sessions: SharedReviewSessions, pub(crate) deployment: DeploymentConfig, pub(crate) wiki_registry: WikiRegistry, pub(crate) wikitext_editor: Arc, diff --git a/crates/sp42-server/src/tests.rs b/crates/sp42-server/src/tests.rs index f8a497a3..53768ace 100644 --- a/crates/sp42-server/src/tests.rs +++ b/crates/sp42-server/src/tests.rs @@ -100,6 +100,7 @@ fn test_state() -> AppState { capability_targets: CapabilityProbeTargets::default(), clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -1158,6 +1159,7 @@ async fn healthz_reports_ready_when_local_token_is_loaded() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -1667,6 +1669,7 @@ async fn capability_route_uses_injected_targets() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -1735,6 +1738,7 @@ async fn live_operator_route_returns_canonical_operator_contract() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -1826,6 +1830,7 @@ async fn live_operator_route_surfaces_cached_backlog_state() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -2002,6 +2007,7 @@ async fn logical_storage_document_route_resolves_profile_page() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -2085,6 +2091,7 @@ async fn public_storage_document_route_returns_typed_preferences() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -2174,6 +2181,7 @@ async fn bootstrap_derives_username_and_scopes_from_validated_token() { }, clock: clock.clone(), coordination: CoordinationRegistry::new(clock), + review_sessions: crate::review_routes::new_review_session_store(), deployment: test_deployment(), wiki_registry: test_wiki_registry(), wikitext_editor: test_wikitext_editor(), @@ -3984,3 +3992,501 @@ fn action_error_response_preserves_carried_status() { axum::http::StatusCode::BAD_GATEWAY ); } + +/// POST a JSON body to a review route with the bridge cookie + CSRF header +/// and return the status plus parsed JSON body. +async fn post_review_json( + router: Router, + cookie: &str, + path: &str, + body: &serde_json::Value, +) -> (StatusCode, serde_json::Value) { + let response = router + .oneshot( + Request::builder() + .method(Method::POST) + .uri(path) + .header(axum::http::header::COOKIE, cookie) + .header(CSRF_HEADER_NAME, "csrf-token") + .header(axum::http::header::CONTENT_TYPE, "application/json") + .body(Body::from(body.to_string())) + .expect("request should build"), + ) + .await + .expect("review request should succeed"); + let status = response.status(); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("review response body should read"); + let value = serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null); + (status, value) +} + +#[tokio::test] +async fn review_routes_require_an_authenticated_bridge_session() { + let state = test_state(); + let router = build_router(state); + + let response = router + .oneshot( + Request::builder() + .method(Method::POST) + .uri(sp42_core::routes::DEV_REVIEW_POLL_PATH) + .header(axum::http::header::CONTENT_TYPE, "application/json") + .body(Body::from( + serde_json::json!({"wiki_id": "frwiki", "title": "Exemple", "wait_ms": 1}) + .to_string(), + )) + .expect("request should build"), + ) + .await + .expect("poll request should succeed"); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +/// A router with an installed bridge session, ready for review requests. +async fn review_test_router() -> (Router, String) { + let state = test_state(); + let created_at_ms = now_ms(); + state.sessions.write().await.insert( + "review-agent".to_string(), + test_session("Reviewer", "secret-token", created_at_ms), + ); + ( + build_router(state), + format!("{SESSION_COOKIE_NAME}=review-agent"), + ) +} + +#[tokio::test] +async fn review_session_loop_delivers_operator_feedback_to_the_agent() { + let (router, cookie) = review_test_router().await; + + // Open on a pasted URL: the title unwraps and the pinned rev is recorded. + let (status, open) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "target": "https://fr.wikipedia.org/wiki/Exemple", + "rev_id": 42, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(open["session"]["title"], "Exemple"); + assert_eq!(open["session"]["rev_id"], 42); + assert_eq!(open["session"]["status"], "open"); + assert_eq!(open["contract_version"], 1); + + // No feedback yet: a bounded poll reports waiting. + let poll_body = serde_json::json!({"wiki_id": "frwiki", "title": "Exemple", "wait_ms": 1}); + let (status, poll) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &poll_body, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "waiting"); + + // The operator queues an anchored prompt and ends the session in one action. + let (status, queued) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_PROMPTS_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "prompts": [{ + "kind": "text", + "prompt": "this quote is not in the source", + "anchor": {"block_ordinal": 3, "ref_id": "cite_ref-a_1-0", "selected_text": "quote"}, + }], + "end_session": true, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(queued["queued"], 1); + + // Feedback queued before the end still delivers, flagged as the final batch. + let (status, poll) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &poll_body, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "feedback"); + assert_eq!(poll["session_ended"], true); + assert_eq!(poll["ended_by"], "operator"); + assert_eq!(poll["prompts"][0]["anchor"]["block_ordinal"], 3); + + // A later poll reports the operator end with reopen etiquette. + let (status, poll) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &poll_body, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "ended"); + assert!( + poll["next_step"] + .as_str() + .expect("next_step should be a string") + .contains("do not reopen"), + ); +} + +#[tokio::test] +async fn review_mutations_ring_the_coordination_room() { + let state = test_state(); + state.sessions.write().await.insert( + "review-agent".to_string(), + test_session("Reviewer", "secret-token", now_ms()), + ); + // A panel is listening on the wiki's room before the agent acts. + let mut room = state.coordination.subscribe("frwiki").await; + let router = build_router(state); + let cookie = format!("{SESSION_COOKIE_NAME}=review-agent"); + + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let envelope = tokio::time::timeout(Duration::from_secs(5), room.recv()) + .await + .expect("open should ring the room promptly") + .expect("room channel should stay open"); + assert_eq!(envelope.sender_id, crate::review_routes::SERVER_SENDER_ID); + let message = + sp42_coordination::decode_message(&envelope.payload).expect("signal should decode"); + let sp42_coordination::CoordinationMessage::ReviewSignal(signal) = message else { + panic!("expected a review signal, got {message:?}"); + }; + assert_eq!(signal.wiki_id, "frwiki"); + assert_eq!(signal.session.title, "Exemple"); + assert_eq!(signal.session.rev_id, 42); + + // Queueing feedback rings again, carrying the pending count so a panel + // can badge without a round-trip. + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_PROMPTS_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "prompts": [{"kind": "message", "prompt": "check the lede"}], + "end_session": false, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + let envelope = tokio::time::timeout(Duration::from_secs(5), room.recv()) + .await + .expect("queue should ring the room promptly") + .expect("room channel should stay open"); + let message = + sp42_coordination::decode_message(&envelope.payload).expect("signal should decode"); + let sp42_coordination::CoordinationMessage::ReviewSignal(signal) = message else { + panic!("expected a review signal, got {message:?}"); + }; + assert_eq!(signal.session.pending_prompts, 1); + + // Draining the feedback rings once more so the panel's badge does not + // sit on the stale pending count. + let (status, poll) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &serde_json::json!({"wiki_id": "frwiki", "title": "Exemple", "wait_ms": 1}), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "feedback"); + let envelope = tokio::time::timeout(Duration::from_secs(5), room.recv()) + .await + .expect("a drained poll should ring the room promptly") + .expect("room channel should stay open"); + let message = + sp42_coordination::decode_message(&envelope.payload).expect("signal should decode"); + let sp42_coordination::CoordinationMessage::ReviewSignal(signal) = message else { + panic!("expected a review signal, got {message:?}"); + }; + assert_eq!(signal.session.pending_prompts, 0); + assert_eq!(signal.session.status, sp42_core::ReviewSessionStatus::Open); +} + +#[tokio::test] +async fn review_queue_refuses_an_ended_session_and_replies_surface_in_open() { + let (router, cookie) = review_test_router().await; + + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + + // The agent posts a summary; the operator surface must be able to read + // it back (an invisible reply would be write-only). + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_REPLY_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "text": "opened — start with the lede", + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + let (status, open) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(open["chat"][0]["role"], "agent"); + assert_eq!(open["chat"][0]["text"], "opened — start with the lede"); + + // The operator ends the session; a later plain queue must not silently + // flip it back to feedback around the reopen gate. + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_PROMPTS_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "prompts": [], + "end_session": true, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + let (status, refused) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_PROMPTS_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "prompts": [{"kind": "message", "prompt": "too late"}], + "end_session": false, + }), + ) + .await; + assert_eq!(status, StatusCode::CONFLICT); + assert_eq!(refused["code"], "review-session-ended"); + + // A nonblocking poll (explicit wait_ms: 0) reports the end immediately. + let (status, poll) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &serde_json::json!({"wiki_id": "frwiki", "title": "Exemple", "wait_ms": 0}), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "ended"); +} + +#[tokio::test] +async fn review_findings_attach_and_overlay_the_next_open() { + let (router, cookie) = review_test_router().await; + + // Attaching to a page with no session refuses. + let marker = serde_json::json!({ + "ref_id": "cite_ref-a_1-0", + "verdict": "not_supported", + "claim": "Cats bark.", + }); + let attach_body = serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "rev_id": 42, + "findings": [marker], + }); + let (status, refused) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_FINDINGS_PATH, + &attach_body, + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!(refused["code"], "review-session-not-found"); + + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + + // A report produced against another revision must not overlay this one. + let mut stale = attach_body.clone(); + stale["rev_id"] = serde_json::json!(41); + let (status, refused) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_FINDINGS_PATH, + &stale, + ) + .await; + assert_eq!(status, StatusCode::CONFLICT); + assert_eq!(refused["code"], "review-findings-revision-mismatch"); + + let (status, attached) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_FINDINGS_PATH, + &attach_body, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(attached["attached"], 1); + assert_eq!(attached["session"]["findings"], 1); + + // The next open carries the overlay. The scripted editor exposes no + // blocks, so the marker surfaces as unanchored rather than dropping; + // block-level joins are covered by the platform unit tests. + let (status, open) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(open["unanchored_findings"][0]["ref_id"], "cite_ref-a_1-0"); + assert_eq!(open["unanchored_findings"][0]["verdict"], "not_supported"); + assert!( + open["next_step"] + .as_str() + .expect("next_step should be a string") + .contains("1 verification finding"), + ); +} + +#[tokio::test] +async fn review_reopen_gate_requires_explicit_reopen_after_operator_end() { + let (router, cookie) = review_test_router().await; + + // Open, then let the operator end the session with no further feedback. + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({"wiki_id": "frwiki", "target": "Exemple", "rev_id": 42}), + ) + .await; + assert_eq!(status, StatusCode::OK); + let (status, _) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_PROMPTS_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "title": "Exemple", + "prompts": [], + "end_session": true, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + // A plain reopen refuses; an explicit reopen resumes. + let open_body = serde_json::json!({ + "wiki_id": "frwiki", + "target": "Exemple", + "rev_id": 42, + }); + let (status, refused) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &open_body, + ) + .await; + assert_eq!(status, StatusCode::CONFLICT); + assert_eq!(refused["code"], "review-session-operator-ended"); + + let (status, reopened) = post_review_json( + router.clone(), + &cookie, + sp42_core::routes::DEV_REVIEW_OPEN_PATH, + &serde_json::json!({ + "wiki_id": "frwiki", + "target": "Exemple", + "rev_id": 42, + "reopen": true, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(reopened["session"]["status"], "open"); + + // The listing shows the resumed session. + let response = router + .oneshot( + Request::builder() + .method(Method::GET) + .uri(sp42_core::routes::DEV_REVIEW_SESSIONS_PATH) + .body(Body::empty()) + .expect("request should build"), + ) + .await + .expect("sessions request should succeed"); + assert_eq!(response.status(), StatusCode::OK); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("sessions body should read"); + let sessions: serde_json::Value = + serde_json::from_slice(&bytes).expect("sessions body should parse"); + assert_eq!(sessions["sessions"][0]["title"], "Exemple"); +} + +#[tokio::test] +async fn review_poll_reports_missing_for_an_unopened_page() { + let state = test_state(); + let created_at_ms = now_ms(); + state.sessions.write().await.insert( + "review-agent".to_string(), + test_session("Reviewer", "secret-token", created_at_ms), + ); + let router = build_router(state); + let cookie = format!("{SESSION_COOKIE_NAME}=review-agent"); + + let (status, poll) = post_review_json( + router, + &cookie, + sp42_core::routes::DEV_REVIEW_POLL_PATH, + &serde_json::json!({"wiki_id": "frwiki", "title": "Nowhere", "wait_ms": 1}), + ) + .await; + + assert_eq!(status, StatusCode::OK); + assert_eq!(poll["status"], "missing"); +} diff --git a/docs/domains/references/README.md b/docs/domains/references/README.md index 04bab5dc..d033533d 100644 --- a/docs/domains/references/README.md +++ b/docs/domains/references/README.md @@ -21,6 +21,7 @@ Operational guidance for the model panel is in [model-panel.md](model-panel.md). - [PRD-0010 — Citation-verification agent surface (MCP)](prd/0010-citation-verification-mcp-surface.md) — expose probe/verify verbs to agent clients via the `sp42-mcp` shell - [PRD-0012 — Citation insertion for unsourced claims](prd/0012-citation-insertion.md) — ground a candidate source against a sentence's atomic claims, then propose a `` insert - [PRD-0014 — Citation repair and insertion, browser surface](prd/0014-citation-repair-insertion-browser-surface.md) — per-finding action row in the Citations tab routing to text-edit or citation-fix, operator always chooses +- [PRD-0017 — Interactive review sessions](prd/0017-interactive-review-sessions.md) — agent opens a session on a page, the operator queues anchored feedback, the agent polls for it (loop ported from lavish-axi, keyed to wiki pages); attached verify-page findings overlay the outline as the report's in-article view ## Architecture Decision Records @@ -29,6 +30,7 @@ Operational guidance for the model panel is in [model-panel.md](model-panel.md). - [ADR-0009 — Source-snapshot storage for reproducibility](adr/0009-citation-source-snapshot-storage.md) - [ADR-0011 — Article-level citation verification (the review path)](adr/0011-article-citation-verification.md) - [ADR-0015 — Rules-compliant read-only fetch edge](adr/0015-rules-compliant-read-only-fetch-edge.md) +- [ADR-0018 — Review-session bridge contract and store placement](adr/0018-review-session-bridge-contract.md) - [ADR-0019 — Cached page re-verification](adr/0019-cached-page-reverification.md) — activate ADR-0009's dormant verdict cache so Re-verify refreshes the whole page to the current revision cheaply The shared LLM posture these depend on (model panel, measured agreement, keyless diff --git a/docs/domains/references/adr/0018-review-session-bridge-contract.md b/docs/domains/references/adr/0018-review-session-bridge-contract.md new file mode 100644 index 00000000..a1f3e26f --- /dev/null +++ b/docs/domains/references/adr/0018-review-session-bridge-contract.md @@ -0,0 +1,149 @@ +# ADR-0018: Review-session bridge contract and store placement + +**Status:** Proposed +**Date:** 2026-07-11 +**Author:** Claude Code (Fable), for Luis Villa + +PRD-0017 ports the agent↔human artifact-review loop popularized by +[lavish-axi](https://github.com/kunchenguid/lavish-axi) from local HTML files +to wiki pages. This ADR records the structural decisions: where the contract +lives, how sessions are keyed, how the long-poll is implemented, and how the +routes are gated. + +## Context + +The ported loop has three parties — an agent (CLI/MCP), an operator surface +(browser), and a rendezvous server — and one contract: session state plus a +queued-feedback drain with strict etiquette (deliver-before-ended, operator +ends gate reopens, `next_step` guidance in every response). lavish-axi keys +sessions by canonical file path, persists them in `~/.lavish-axi/state.json`, +watches the file for live reload, and long-polls one indefinite HTTP request +with whitespace heartbeats. None of the file-specific machinery maps to a +remote wiki page; the loop semantics map cleanly. + +SP42 constraints that shaped the port: contracts shared by more than one crate +live in `sp42-platform` (ADR-0013); business logic is pure and tested with no +I/O (Constitution Art. 1–2); the localhost server's mutable stores are +`Arc>` fields on `AppState` with session+CSRF gating on +mutating routes (ADR-0002); external interfaces carry versioned serde +contracts (Art. 9.1). + +## Decision + +1. **Pure core in `sp42-platform::review_session`.** `ReviewSession` owns + every transition — `queue_prompts` (which refuses an ended session so a + stale queue cannot flip it back to feedback around the reopen gate), + `take_feedback` (deliver-before-ended, drain-exactly-once), `gate_reopen` + (operator-ended refuses a plain reopen; the route checks this against + the stored session *before* any remote revision/Parsoid read), `resume`, + `end`, `agent_reply` (chat is echoed back on the open response so + replies are not write-only) — as pure methods over injected `now_ms`. The wire types (`ReviewOpenRequest/Response`, `ReviewPollRequest/ + Response`, `ReviewQueueRequest/Response`, ack/list shapes) live beside + them, all embedding `REVIEW_SESSION_CONTRACT_VERSION`. The `next_step` + strings are pure functions too, so the loop etiquette is unit-tested. + +2. **Session identity is the parsed `(wiki_id, title)` pair.** The open + route accepts a bare title or a pasted wiki URL and unwraps it with the + existing `parse_page_target` (same as verify-page), so every URL spelling + of a page collapses to one session — the analog of lavish-axi's + canonical-path key. The store key is `wiki_id ␟ title` (unit-separator + joined, so the pair is unambiguous), and `canonical_key` applies the + wiki-independent half of MediaWiki title normalization to the title + component — underscores are spaces, whitespace runs collapse — so bare + titles, URL spellings, and direct API callers all key one session. + First-letter case is deliberately *not* folded: case rules are + wiki-dependent (Wiktionary titles are case-sensitive), so folding could + merge two genuinely distinct pages; the residual cost is that a + lowercase-typed title opens a separate session on case-insensitive + wikis. Full API-side normalization (case, namespace aliases) would need + a wiki round-trip per request and is out of scope for the dev bridge. + The revision is session *state* (pinned at open, re-pinned on resume), + not identity. + +3. **Server store: `AppState.review_sessions`, in-memory, with a per-session + `tokio::sync::Notify`.** Each entry pairs the pure `ReviewSession` with a + `Notify`; queue/end call `notify_one`, whose stored permit covers feedback + queued while no poll is waiting. The poll handler drains, waits + (`tokio::time::timeout` over `notified()`, clamped to 55 s), and drains + once more; an omitted `wait_ms` uses the server default, while an + explicit `wait_ms: 0` skips the wait — a nonblocking status check. Bounded server-side waits with a re-arming CLI loop replace + lavish-axi's single indefinite request — no heartbeat protocol, no + proxy-timeout exposure, identical agent ergonomics. No cross-restart + persistence: this is a localhost dev-bridge surface (PRD-0017 Risks). + +4. **Routes under `/dev/review/*`, POSTs session+CSRF gated.** Path constants + in `sp42_platform::routes` like every other route contract. All six POST + routes (`open`, `prompts`, `poll`, `findings`, `agent-reply`, `end`) require the + bridge session cookie plus the CSRF header (ADR-0002) — open reads through + the caller's wiki identity to pin the latest revision, and the rest mutate + session state. `GET /dev/review/sessions` is an ungated read-only + inspection surface, listed in the operator endpoint manifest. + +5. **The article outline replaces the DOM snapshot.** Where lavish-axi ships + a walked-DOM outline with synthetic uids, the open response ships + `build_article_outline` over the Parsoid blocks the editor already + extracts (`extract_blocks`, ADR-0011): block ordinal, kind, truncated + text, cite ids. Prompt anchors reuse the same coordinates + (`block_ordinal` + optional `ref_id` + optional verbatim `selected_text`), + which map directly onto citation use-sites — no CSS selectors. + +6. **Agent surface is `sp42-cli review …` over the bridge-bootstrap pattern.** + Open/poll/queue/reply/end/sessions subcommands authenticate exactly like + `verify-page` (bootstrap → cookie + CSRF token). The poll loop keeps + stdout reserved for one structured response and narrates waiting on + stderr, mirroring the ported tool's agent ergonomics. MCP verbs are + deferred (PRD-0017 open question 2) because `sp42-mcp` has no + localhost-server client today. + +7. **The verification report overlays the outline as citation-agnostic + markers.** `sp42-platform` gains `ReviewFindingMarker` — `ref_id`, + verdict wire label, truncated claim, optional caveat — deliberately *not* + the citation domain's `CitationFinding`: the platform layer never depends + on report types (ADR-0013 layering), so the citation crate projects its + report into markers at the edge (`review_finding_markers`, which also + translates grounding caveats and unavailable reasons into prose and skips + ref-less standalone findings). Markers attach via `POST + /dev/review/findings` with replace-all semantics (one attach = one + verification run) and a revision gate — a report pinned to a different + revision returns 409 `review-findings-revision-mismatch`, and resuming a + session onto a new revision drops the stale overlay. On open, the pure + `annotate_outline` joins markers onto blocks by `ref_id` (the same + coordinate prompts anchor to); unmatched markers return as + `unanchored_findings` rather than dropping. Attaching wakes no poll — + findings are operator-surface input, not agent feedback. + +8. **Live panel updates ride the coordination room as an advisory signal — + the substrates stay separate.** The review store and the coordination + substrate make opposite delivery guarantees: review feedback is a + durable, drain-exactly-once queue keyed per page; coordination is + live-only state sync keyed per wiki with no backfill (ADR-0023 §2). + Folding one into the other would break one contract or the other, so + they converge only at the notification layer: every review mutation + (open, queue, findings, reply, end) publishes a `ReviewSignal` — a + seventh `CoordinationMessage` kind extending ADR-0023's registry — to + the wiki's room with the server sender id (`0`, never a client id, so + it is never echo-suppressed). The signal is a doorbell, not the data: + it carries only the session snapshot for badging, the reducer relays + it without folding any room state, and panels re-fetch through the + gated review routes, which remain the sole source of truth. This makes + spoofing harmless (a client-forged signal can only cause a re-fetch), + keeps the agent side on the long-poll (no WebSocket client in + `sp42-cli`/`sp42-mcp`), and loses nothing when no operator is in the + room — an unheard doorbell has no one to notify, and a late-joining + panel lists sessions over REST. + +## Consequences + +- The loop's semantics are testable without a server, and the server tests + exercise only glue (gating, wake-ups, status codes). +- A browser Review panel needs no new contract — it consumes the same routes + the CLI queueing surface uses. +- The text report is no longer the only operator surface for verification + results: any consumer of the open response gets the report as an + in-article, per-block overlay for free. +- Session state dies with the server process; if review sessions outgrow the + dev bridge (multi-operator, cross-restart), persistence and eviction become + a follow-up ADR alongside the deployment-mode gating question. +- The bounded-wait poll means worst-case feedback latency for a poll that + raced a notification is one wait window; the permit semantics make that + race a non-event in the single-agent case. diff --git a/docs/domains/references/prd/0017-interactive-review-sessions.md b/docs/domains/references/prd/0017-interactive-review-sessions.md new file mode 100644 index 00000000..658065ea --- /dev/null +++ b/docs/domains/references/prd/0017-interactive-review-sessions.md @@ -0,0 +1,187 @@ +# PRD-0017: Interactive review sessions — agent↔operator feedback loop on a page + +**Drafter:** Claude Code (Fable) +**Editor:** Luis Villa +**Date:** 2026-07-11 +**State:** Draft +**Discussion:** +**Spawned ADRs:** [ADR-0018](../adr/0018-review-session-bridge-contract.md) +(review-session bridge contract and store placement) + +## Problem + +An agent working on citations (via `sp42-cli` or `sp42-mcp`) and an operator +reviewing an article currently have no shared, structured feedback loop. The +operator's judgment reaches the agent as free-form chat: "the third paragraph's +quote isn't in the source" — no page identity, no revision, no block, no cite +id. The agent guesses at anchors, and the operator repeats themselves. + +Local-first agent review tools solved this loop for *files*: +[lavish-axi](https://github.com/kunchenguid/lavish-axi) keys a session to an +HTML artifact's canonical path, lets the human annotate elements and text +ranges in a browser, and delivers the queued, anchored prompts to whichever +agent polls the session. The loop etiquette it ships — deliver queued feedback +before reporting an ended session, refuse to reopen a session the *human* +ended unless explicitly asked, tell the agent its next step in every response +— is what makes the loop usable by agents without bespoke prompting. + +SP42 wants that loop, keyed to what SP42 reviews: a **wiki page**, identified +by `(wiki_id, title)` and pinned to a revision — passing a Wikipedia URL where +lavish-axi passes a file path. + +## Proposal + +A localhost **review-session bridge**: the agent opens a session on a page +target (bare title or pasted wiki URL, including `oldid` URLs), the operator +queues anchored feedback prompts against that page, and the agent polls the +session to collect them. + +User-facing behavior: + +- **Open**: `sp42-cli review open ` opens (or resumes) the + session and returns a compact **article outline** — block ordinals, block + kinds, truncated text, and cite ids from the Parsoid decomposition — the + agent's map for resolving anchors, plus a `next_step` telling it to poll. +- **Annotate**: the operator queues prompts anchored to a block ordinal, + optionally narrowed to a cite id (`cite_ref-…`) or a verbatim selected-text + range, or free-form messages. Anchors use Parsoid structure, not CSS + selectors, so they survive re-rendering and map directly onto the + use-site anchors the verification domain already uses. The MVP queueing + surface is `sp42-cli review queue` (dev/test) and the gated + `/dev/review/prompts` route; the browser annotation panel is a committed + follow-up (see Open questions). +- **Poll**: `sp42-cli review poll ` waits for feedback. Each + server request is a bounded wait; the CLI re-arms until feedback, an end, + or a missing session, narrating the wait on stderr while stdout stays a + single structured response. `--agent-reply ""` posts a chat line + to the operator surface before the wait starts. +- **Findings overlay**: the session doubles as the *in-article frontend for + the verification report*. `sp42-cli review findings + --report ` attaches a `PageVerificationReport`'s + findings to the session, projected onto the same anchors prompts use — + each finding joins its outline block by cite id, carrying the verdict, a + truncated claim, and a caveat (dead source, ungrounded support). The next + open returns the annotated outline, so the operator sees where the + problems are in the article instead of reading a detached text report, + and can queue prompts directly against them. Markers matching no block + surface as `unanchored_findings`, never silently dropped; a report + produced against a different revision than the session's pin is refused + (`review-findings-revision-mismatch`). Attaching spends no inference — + verification stays operator-triggered (PRD-0014 posture); the agent runs + `verify-page` explicitly and hands the result over. +- **Session-end etiquette** (ported): feedback queued before an end still + delivers first, flagged `session_ended`; only a later poll reports `ended`. + An **operator**-ended session refuses a plain reopen (HTTP 409 with + guidance) unless `--reopen` is passed; an **agent**-ended session + (`sp42-cli review end`) reopens freely. An ended session also refuses new + prompts (`review-session-ended`) until it is explicitly opened again — a + stale queue must not flip a closed session back to feedback around the + gate. Every response carries `next_step` prose telling the agent what to + do — poll again, apply-and-reply, or stop without reopening. +- **Inventory**: `sp42-cli review sessions` and `GET /dev/review/sessions` + list open sessions with pending-prompt counts. +- **Live signal**: every session mutation rings the wiki's coordination + room with an advisory `ReviewSignal` (session snapshot only — a doorbell, + not the data), so a browser panel gets push updates over the existing + WebSocket without a second push transport; it re-fetches the review + routes, which stay the source of truth (ADR-0018 §8). + +Everything stays read-only with respect to the wiki: a review session never +edits anything. Acting on feedback rides the existing operator-confirmed +lanes (re-verify, bare-URL repair, inline edit). + +## Definition of Done + +- [ ] Opening with a pasted wiki URL unwraps the title and records the pinned + revision, verified by `review_session_loop_delivers_operator_feedback_to_the_agent` + (`sp42-server`) +- [ ] Prompts queued before an operator "send & end" deliver on the next poll + flagged `session_ended`, and only the following poll reports `ended`, + verified by `feedback_queued_before_an_end_delivers_before_ended` + (`sp42-platform`) and the server loop test +- [ ] A plain open of an operator-ended session refuses with + `review-session-operator-ended` and an explicit `reopen` resumes it, + verified by `review_reopen_gate_requires_explicit_reopen_after_operator_end` + (`sp42-server`) +- [ ] Feedback drains exactly once — a second poll after delivery reports + `waiting`, verified by `take_feedback_drains_once_and_reopens` + (`sp42-platform`) +- [ ] Queueing wakes a concurrently waiting poll without a full timeout wait, + verified by `queued_feedback_wakes_a_waiting_poll` (`sp42-server`) +- [ ] All review POST routes require an authenticated bridge session and CSRF + header, verified by `review_routes_require_an_authenticated_bridge_session` + (`sp42-server`) +- [ ] Every response embeds `contract_version` and a `next_step`, and the + ended `next_step` distinguishes operator-ended (do not reopen) from + agent-ended, verified by `next_step_matches_the_loop_etiquette` + (`sp42-platform`) +- [ ] Attached findings overlay the next open's outline joined by cite id, + with unmatched markers surfaced (not dropped), verified by + `annotate_outline_joins_findings_onto_blocks_by_ref_id` (`sp42-platform`) + and `review_findings_attach_and_overlay_the_next_open` (`sp42-server`) +- [ ] A report for a different revision than the session's pin refuses with + `review-findings-revision-mismatch`, and resuming to a new revision drops + the stale overlay, verified by `attach_findings_refuses_a_revision_mismatch` + and `resume_to_a_new_revision_drops_stale_findings` (`sp42-platform`) +- [ ] Report findings project onto review anchors — verdict wire labels, + truncated claims, grounding/unavailable caveats, ref-less standalone + findings skipped — verified by + `review_finding_markers_project_the_report_onto_review_anchors` + (`sp42-citation`) +- [ ] Queueing to an ended session refuses with `review-session-ended`, and + agent chat replies are readable back on the open response, verified by + `queueing_to_an_ended_session_is_refused` (`sp42-platform`) and + `review_queue_refuses_an_ended_session_and_replies_surface_in_open` + (`sp42-server`) +- [ ] Every review mutation publishes an advisory `ReviewSignal` to the + wiki's coordination room, relayed without folding room state, verified by + `review_mutations_ring_the_coordination_room` (`sp42-server`) and + `review_signal_relays_without_folding_room_state` (`sp42-coordination`) +- [ ] Session opens, feedback queueing, findings attaches, deliveries, and + ends emit `tracing` events, checkable in server logs + +## Alternatives + +- **Adopt lavish-axi itself** (open the article as a local HTML artifact): + rejected — its identity model (canonical file path, chokidar file watching, + sibling-asset serving) does not map to remote wiki URLs; its annotation + anchors (truncated CSS selectors + DOM ranges) are weaker than Parsoid + block/cite anchors for wiki work; and SP42 already owns the localhost + server, session/CSRF runtime, and Parsoid read path the loop needs. +- **WebSocket feedback channel** (move the whole loop onto the + coordination-room substrate): rejected — a long-poll matches the agent's + blocking-command ergonomics (one CLI invocation = one rendezvous) and + needs no client protocol, and the substrates' delivery guarantees are + opposites (durable exactly-once feedback queue vs. live-only no-backfill + state sync, ADR-0023). Instead the browser side converges at the + notification layer only: every review mutation rings the wiki's + coordination room with an advisory `ReviewSignal` the panel answers by + re-fetching the review routes (ADR-0018 §8). +- **Key sessions by URL string** (hash the raw URL like lavish-axi hashes + paths): rejected — two URLs for the same page (percent-encoding, + underscores, `oldid` forms) must resolve to one session, so the canonical + key is the parsed `(wiki_id, title)` pair. + +## Risks + +- **The MVP has no browser annotation surface**, so operator queueing runs + through the CLI/route until the panel ships — the loop is real but the + operator ergonomics are not yet. Mitigation: the wire contract is versioned + and panel-agnostic; the browser panel consumes the same routes. +- **Feedback is held in server memory only**; a server restart drops queued + prompts, unlike lavish-axi's `state.json`. Acceptable for a localhost dev + bridge; revisit persistence when multi-operator review sessions arrive. +- **An agent could ignore the reopen etiquette** and hammer `reopen: true`. + The gate is advisory by design (same as the ported tool); the operator + surface always shows who reopened. + +## Open questions + +1. **Where does the operator annotate?** Proposed: a Review panel in the + browser shell's article surface, listing outline blocks — with attached + findings rendered inline as per-block badges — plus per-block queue + actions and a session composer, reusing the `/dev/review` routes. + Until then the CLI `review queue` command is the queueing surface. +2. **MCP surface**: should `sp42-mcp` grow `review_open`/`review_poll` verbs? + Proposed: yes, but the MCP server is standalone today (no localhost-server + client); bridging it is its own small ADR. diff --git a/docs/platform/CLI.md b/docs/platform/CLI.md index da81e06a..9f57fe72 100644 --- a/docs/platform/CLI.md +++ b/docs/platform/CLI.md @@ -33,6 +33,7 @@ quick one. | `batch` | Verify a JSONL batch (one case per line), one result line out. | JSONL cases (or `--file`) | | `locate-probe` | Offline check whether a quote locates in a source body. | source body | | `bare-url preview` / `bare-url execute` | Preview or apply bare-URL repairs. | no | +| `review ` | Interactive review sessions: open a page, wait for operator feedback. | no | | `preview [mode]` | Dev / operator queue views; default is the ranked queue. | event payload | Output goes to **stdout** on success (exit `0`); errors print to **stderr** @@ -209,6 +210,38 @@ sp42-cli bare-url execute --title "Museum" --rev 12345 --ordinal 0 | `--action-note ` | Edit summary attached to the applied repair. | | `--bridge-base-url ` | Local server base URL. | +## `review` — interactive review sessions (PRD-0017) + +Agent↔operator feedback loop on a page: the agent opens a session on a page +target (bare title or pasted wiki URL, including `oldid` URLs), the operator +queues anchored feedback prompts, and the agent polls until they arrive. All +actions ride the local bridge session (bootstrap → cookie + CSRF). + +```sh +sp42-cli review open "https://fr.wikipedia.org/wiki/Exemple" --wiki frwiki +sp42-cli review poll "Exemple" --wiki frwiki --agent-reply "opened — start with the lede" +sp42-cli review queue "Exemple" --wiki frwiki --message "check this ref" --block 3 --ref-id cite_ref-a_1-0 +sp42-cli verify-page --title "Exemple" --wiki frwiki --format json > report.json +sp42-cli review findings "Exemple" --wiki frwiki --report report.json +sp42-cli review end "Exemple" --wiki frwiki +sp42-cli review sessions +``` + +| Action | What it does | +| --- | --- | +| `open ` | Open or resume a session; returns the article outline plus `next_step`. `--rev ` pins a revision (default: latest), `--reopen` resumes a session the operator explicitly ended. | +| `poll ` | Wait for feedback; re-arms bounded server waits until feedback, an end, or a missing session (stderr narrates the wait). `--agent-reply ` posts a chat line first, `--once` returns after a single wait. | +| `queue ` | Queue one operator prompt (dev/test surface): `--message ` plus optional anchors `--block `, `--ref-id `, `--selected-text `; `--end` sends and ends in one action. | +| `findings ` | Attach a `verify-page --format json` report (`--report `, `-` for stdin) so its findings overlay the outline by cite id — the report's in-article view. The report must describe the target page/wiki, and is refused when its revision differs from the session's (`review-findings-revision-mismatch`). | +| `reply ` | Send an agent chat reply (`--message `). | +| `end ` | End the session as the agent; a plain reopen stays allowed. | +| `sessions` | List review sessions on the local server. | + +All actions accept `--wiki ` (default `testwiki`), `--bridge-base-url +`, and `--format `. An operator-ended session refuses a plain +`open` with `review-session-operator-ended`; pass `--reopen` only when the +operator asks for further review. + ## `preview` — dev / operator views Consumes an event payload from STDIN (or a built-in sample when STDIN is empty) diff --git a/docs/platform/architecture.md b/docs/platform/architecture.md index d4b81aa7..aae1b76b 100644 --- a/docs/platform/architecture.md +++ b/docs/platform/architecture.md @@ -50,10 +50,10 @@ flowchart LR flowchart TB subgraph SHELLS["Shells — composition roots"] sp42_app["sp42-app
ADR-0004 … +5 more ADRs"]:::shell - sp42_cli["sp42-cli
ADR-0004 … +3 more ADRs"]:::shell + sp42_cli["sp42-cli
ADR-0004 … +4 more ADRs"]:::shell sp42_desktop["sp42-desktop
ADR-0004, ADR-0005, ADR-0013"]:::shell sp42_devtools["sp42-devtools
ADR-0004, ADR-0013"]:::shell - sp42_mcp["sp42-mcp
ADR-0016"]:::shell + sp42_mcp["sp42-mcp
ADR-0016, ADR-0018"]:::shell sp42_server["sp42-server
ADR-0003 … +9 more ADRs"]:::shell end subgraph DOMAINS["Domains — policy, config, workflow"] @@ -76,7 +76,7 @@ flowchart TB sp42_inference["sp42-inference
ADR-0011, ADR-0013, ADR-0015"]:::platform sp42_live["sp42-live
ADR-0004, ADR-0013, ADR-0020"]:::platform sp42_parsoid["sp42-parsoid"]:::platform - sp42_platform["sp42-platform
ADR-0013 … +6 more ADRs"]:::platform + sp42_platform["sp42-platform
ADR-0013 … +7 more ADRs"]:::platform sp42_reporting["sp42-reporting
ADR-0004 … +4 more ADRs"]:::platform sp42_types["sp42-types
ADR-0004 … +9 more ADRs"]:::platform sp42_ui["sp42-ui
ADR-0005"]:::platform @@ -159,21 +159,21 @@ Reading notes: |---|---|---|---|---| | `sp42-app` | shell | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0010](adr/0010-operator-confirmed-content-proposals.md), [ADR-0012](adr/0012-frontend-e2e-testing-approach.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md) | [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md), [PRD-0010](../domains/references/prd/0010-citation-verification-mcp-surface.md), [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md) | | | `sp42-assessment` | domain | — | [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md) | | -| `sp42-citation` | domain | [ADR-0013](adr/0013-layered-platform-domain-architecture.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0008](../domains/references/prd/0008-bare-url-repair.md), [PRD-0010](../domains/references/prd/0010-citation-verification-mcp-surface.md), [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md), [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md) | | -| `sp42-cli` | shell | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md) | | -| `sp42-coordination` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0023](adr/0023-coordination-contract.md) | [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md) | | +| `sp42-citation` | domain | [ADR-0013](adr/0013-layered-platform-domain-architecture.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0008](../domains/references/prd/0008-bare-url-repair.md), [PRD-0010](../domains/references/prd/0010-citation-verification-mcp-surface.md), [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md), [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md), [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | +| `sp42-cli` | shell | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0018](../domains/references/adr/0018-review-session-bridge-contract.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | +| `sp42-coordination` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0023](adr/0023-coordination-contract.md) | [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md), [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | | `sp42-core` | hybrid | [ADR-0001](adr/0001-foundational-decisions.md), [ADR-0003](adr/0003-node-anchored-wikitext-editing.md), [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0006](adr/0006-using-llms.md), [ADR-0007](../domains/references/adr/0007-citation-verification-semantics.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0009](../domains/references/adr/0009-citation-source-snapshot-storage.md), [ADR-0010](adr/0010-operator-confirmed-content-proposals.md), [ADR-0011](../domains/references/adr/0011-article-citation-verification.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0021](adr/0021-scoring-and-ranking-contract.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0004](../domains/patrolling/prd/0004-reviewer-actions-on-wikimedia.md), [PRD-0005](../domains/patrolling/prd/0005-operator-identity-and-session.md), [PRD-0007](../domains/references/prd/0007-llm-output-benchmarking.md), [PRD-0015](../domains/assessment/prd/0015-article-stability-signal.md), [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md) | Hybrid exemption — re-export facade being split into platform/domain crates and retired ([ADR-0013](adr/0013-layered-platform-domain-architecture.md)) | | `sp42-desktop` | shell | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md) | — | | | `sp42-devtools` | shell | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md) | [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md) | | | `sp42-fetch` | platform | [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0024](../domains/references/adr/0024-open-library-internet-archive-read-contract.md), [ADR-0025](../domains/references/adr/0025-open-library-apply-contract.md) | — | | | `sp42-inference` | platform | [ADR-0011](../domains/references/adr/0011-article-citation-verification.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md) | — | Still depends on `sp42-core`; edge disappears when the facade is retired ([ADR-0013](adr/0013-layered-platform-domain-architecture.md)) | | `sp42-live` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md) | [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0004](../domains/patrolling/prd/0004-reviewer-actions-on-wikimedia.md) | | -| `sp42-mcp` | shell | [ADR-0016](adr/0016-wikidata-entity-content-model.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0010](../domains/references/prd/0010-citation-verification-mcp-surface.md) | | +| `sp42-mcp` | shell | [ADR-0016](adr/0016-wikidata-entity-content-model.md), [ADR-0018](../domains/references/adr/0018-review-session-bridge-contract.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0010](../domains/references/prd/0010-citation-verification-mcp-surface.md), [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | | `sp42-parsoid` | platform | — | — | | | `sp42-patrol` | domain | [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md), [ADR-0021](adr/0021-scoring-and-ranking-contract.md) | — | | -| `sp42-platform` | platform | [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0016](adr/0016-wikidata-entity-content-model.md), [ADR-0017](adr/0017-wikidata-statement-proposal-write-contract.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md), [ADR-0021](adr/0021-scoring-and-ranking-contract.md), [ADR-0022](adr/0022-reviewer-action-contract.md), [ADR-0023](adr/0023-coordination-contract.md) | — | | +| `sp42-platform` | platform | [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0016](adr/0016-wikidata-entity-content-model.md), [ADR-0017](adr/0017-wikidata-statement-proposal-write-contract.md), [ADR-0018](../domains/references/adr/0018-review-session-bridge-contract.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md), [ADR-0021](adr/0021-scoring-and-ranking-contract.md), [ADR-0022](adr/0022-reviewer-action-contract.md), [ADR-0023](adr/0023-coordination-contract.md) | [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | | `sp42-reporting` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0009](../domains/references/adr/0009-citation-source-snapshot-storage.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0016](adr/0016-wikidata-entity-content-model.md) | [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md) | | -| `sp42-server` | shell | [ADR-0003](adr/0003-node-anchored-wikitext-editing.md), [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0009](../domains/references/adr/0009-citation-source-snapshot-storage.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md), [ADR-0022](adr/0022-reviewer-action-contract.md), [ADR-0023](adr/0023-coordination-contract.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0003](../domains/patrolling/prd/0003-edit-scoring-and-queue-ranking.md), [PRD-0004](../domains/patrolling/prd/0004-reviewer-actions-on-wikimedia.md), [PRD-0005](../domains/patrolling/prd/0005-operator-identity-and-session.md), [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md), [PRD-0008](../domains/references/prd/0008-bare-url-repair.md), [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md) | | +| `sp42-server` | shell | [ADR-0003](adr/0003-node-anchored-wikitext-editing.md), [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0009](../domains/references/adr/0009-citation-source-snapshot-storage.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md), [ADR-0022](adr/0022-reviewer-action-contract.md), [ADR-0023](adr/0023-coordination-contract.md) | [PRD-0001](../domains/references/prd/0001-citation-verification.md), [PRD-0002](../domains/patrolling/prd/0002-patrol-review-workflow.md), [PRD-0003](../domains/patrolling/prd/0003-edit-scoring-and-queue-ranking.md), [PRD-0004](../domains/patrolling/prd/0004-reviewer-actions-on-wikimedia.md), [PRD-0005](../domains/patrolling/prd/0005-operator-identity-and-session.md), [PRD-0006](../domains/patrolling/prd/0006-multi-operator-coordination.md), [PRD-0008](../domains/references/prd/0008-bare-url-repair.md), [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md), [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | | | `sp42-types` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0008](../domains/references/adr/0008-citation-verification-contract.md), [ADR-0009](../domains/references/adr/0009-citation-source-snapshot-storage.md), [ADR-0011](../domains/references/adr/0011-article-citation-verification.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md), [ADR-0016](adr/0016-wikidata-entity-content-model.md), [ADR-0017](adr/0017-wikidata-statement-proposal-write-contract.md), [ADR-0023](adr/0023-coordination-contract.md) | — | | | `sp42-ui` | platform | [ADR-0005](adr/0005-design-system-shared-component-layer.md) | — | | | `sp42-wiki` | platform | [ADR-0004](adr/0004-crate-boundary-collaboration-model.md), [ADR-0005](adr/0005-design-system-shared-component-layer.md), [ADR-0013](adr/0013-layered-platform-domain-architecture.md), [ADR-0014](adr/0014-wikimedia-oauth-and-any-project.md) | [PRD-0005](../domains/patrolling/prd/0005-operator-identity-and-session.md) | Still depends on `sp42-core`; edge disappears when the facade is retired ([ADR-0013](adr/0013-layered-platform-domain-architecture.md)) | @@ -199,6 +199,7 @@ Reading notes: | [ADR-0015](../domains/references/adr/0015-rules-compliant-read-only-fetch-edge.md) | Rules-compliant read-only fetch edge | Accepted | references | `sp42-cli`, `sp42-core`, `sp42-fetch`, `sp42-inference`, `sp42-server`, `sp42-types` | | [ADR-0016](adr/0016-wikidata-entity-content-model.md) | Wikidata entity content-model — revision read, `EntityDiff`, and content-model routing | Proposed | platform | `sp42-mcp`, `sp42-platform`, `sp42-reporting`, `sp42-types` | | [ADR-0017](adr/0017-wikidata-statement-proposal-write-contract.md) | Wikidata statement-proposal write contract | Proposed | platform | `sp42-platform`, `sp42-types` | +| [ADR-0018](../domains/references/adr/0018-review-session-bridge-contract.md) | Review-session bridge contract and store placement | Proposed | references | `sp42-cli`, `sp42-mcp`, `sp42-platform` | | [ADR-0019](../domains/references/adr/0019-cached-page-reverification.md) | Cached page re-verification | Proposed | references | — | | [ADR-0020](../domains/patrolling/adr/0020-live-operator-view-contract.md) | Live-operator-view contract — the server-assembled patrol payload | Accepted | patrolling | `sp42-live`, `sp42-patrol`, `sp42-platform`, `sp42-server` | | [ADR-0021](adr/0021-scoring-and-ranking-contract.md) | Scoring and ranking contract — composite score, policy schema, compile step | Accepted | platform | `sp42-core`, `sp42-patrol`, `sp42-platform` | @@ -226,3 +227,4 @@ Reading notes: | [PRD-0014](../domains/references/prd/0014-citation-repair-insertion-browser-surface.md) | Citation repair and insertion — browser surface | Accepted | references | ADR-0003, ADR-0006, ADR-0009, ADR-0010, ADR-0019 | | [PRD-0015](../domains/assessment/prd/0015-article-stability-signal.md) | Article stability signal | Discussion | assessment | ADR-0006, ADR-0007, ADR-0009, ADR-0011, ADR-0014 | | [PRD-0016](../domains/assessment/prd/0016-ga-evidence-appendix-renderer.md) | GA evidence appendix renderer | Implemented (MVP; criterion-5 arm staged on PRD-0015) | assessment | ADR-0003, ADR-0007, ADR-0010, ADR-0011, ADR-0013, ADR-0018 | +| [PRD-0017](../domains/references/prd/0017-interactive-review-sessions.md) | Interactive review sessions — agent↔operator feedback loop on a page | Draft | references | ADR-0018, ADR-0023 |