From 54b13b057d1bc56474150ec7f040f36876d038a5 Mon Sep 17 00:00:00 2001 From: Joel Teply Date: Mon, 17 Aug 2026 10:17:48 -0500 Subject: [PATCH] fix(work): a claimed bench card's solve happens IN THE CARD'S ROOM, and the dispatch takes a TYPED struct (#425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MEASURED FIRST, because the doctrine doc's premise is wrong. Every workspace trace record carries `room_id`; counting nil vs real across the 25 newest trace files: **13,209 roomless turns vs 138,088 roomed (8.7%)**. Per citizen: 90e758b2 8,657 nil / 15,814 roomed — 35% of her cognition happened where no room could see it; a20b3ada 895 nil / 489 roomed, more roomless than roomed. WHAT THE DOC OVERSTATES: "a detached agent/solve produces no turns, so none of it reaches the curriculum." It DOES produce turns. `build_workspace_cycle` attaches a `JsonlWorkspaceCaptureSink` for every cycle — live turn, eval fork and solve alike (persona_workspace.rs:551-564); 1,258 trace files, 6.7 GB on disk. #427 is the corroboration and I had already accepted it: its whole subject was benchmark forks writing to the SAME traces file as live turns. You cannot contaminate a corpus you do not write to. SO THE DEFECT IS NARROWER AND STILL SERIOUS: those turns are invisible to the ROOM. No act receipts radiate (`apply_act` skips a nil room BY DESIGN), so no peer, no human and no ViewState ever sees the work — a citizenship defect, not a curriculum-input one. THE ROOM WAS NEVER UNKNOWN. Boards are PER-ROOM, so a card's activity is the room whose board holds it — and the wrong-room claim retry directly above already resolved exactly that, inline. Extracted as `room_holding_card`: ONE resolver, two consumers, replacing a scan in one place and a shrug in the other. `work/claim` now resolves the card's room and dispatches into it. A card no subscribed board holds gets NO detached fallback — the claim stands and a `work.claim.unplaceable_card` probe says why, because inventing invisible work is the thing being removed. THE SIGNATURE, on Joel's two calls today. "Make sure params are well formed structs and constants, good OOP, not random parameter lists." The old one was `(ctx, airc, Uuid, WorkCardId, Option)` — two BARE UUIDs separated only by argument POSITION, so transposing claimer and room compiled cleanly and would have dispatched a citizen's solve under a room's id. Now `StagedSolveDispatch { claimer: PeerId, card: WorkCardId, room: RoomId }`: typed fields make the transposition a compile error, and the next fact this dispatch needs is a FIELD, not a sixth argument. `ctx` + `airc` stay separate parameters deliberately — ambient services, not facts about the dispatch; folding them in turns a value object into a context bag. "If something is required, remove the option… caught at compile time not runtime." `room` was `Option` and `work/claim` passed `None` — the whole defect, expressed as a type that permitted it. It is now `RoomId`, so the roomless dispatch is UNREPRESENTABLE: a caller that cannot name the activity cannot construct the struct. The fallible part is the LOOKUP, which is where fallibility belongs, and it is handled at the boundary before the struct exists. Adjacent smells named, not silently expanded: the dispatch roster still carries bare `(String, Uuid)` tuples (#396's subject) so benchmark.rs types the id AT this boundary; `AgentSolveParams::room` stays `Option` because `agent/solve` is also operator-invocable, and narrowing that param is its own slice. `cargo check -p continuum-core --features metal,accelerate` clean, zero errors. LIVE PROOF OWED and NOT claimed: no dispatched claim has run on this build yet. The falsifiable prediction is that a claim-fired solve's acts now radiate receipts into the card's room, and that the nil-room share of new traces falls. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo --- core/continuum-core/src/commands/benchmark.rs | 12 +- core/continuum-core/src/modules/work.rs | 176 +++++++++++++----- 2 files changed, 136 insertions(+), 52 deletions(-) diff --git a/core/continuum-core/src/commands/benchmark.rs b/core/continuum-core/src/commands/benchmark.rs index 35b29568f..674598da6 100644 --- a/core/continuum-core/src/commands/benchmark.rs +++ b/core/continuum-core/src/commands/benchmark.rs @@ -1449,9 +1449,15 @@ impl ActionCommand for BenchmarkDispatch { crate::modules::work::dispatch_staged_swe_solve( ctx, &airc, - *who_peer, - card_id, - Some(room.room_id.as_uuid()), + crate::modules::work::StagedSolveDispatch { + // The roster still carries bare `(String, Uuid)` tuples — a + // loose-id smell of its own (#396). Typed at THIS boundary so + // the dispatch cannot confuse a peer with a room; typing the + // roster itself is that card's work, not a silent widening here. + claimer: crate::identity::PeerId::from_uuid(*who_peer), + card: card_id, + room: room.room_id, + }, ) .await; solves_fired += 1; diff --git a/core/continuum-core/src/modules/work.rs b/core/continuum-core/src/modules/work.rs index bbd82b771..46e854ab9 100644 --- a/core/continuum-core/src/modules/work.rs +++ b/core/continuum-core/src/modules/work.rs @@ -302,51 +302,68 @@ fn parse_state(s: &str) -> Result { // ─────────────────────────── work/claim ────────────────────────── -/// Locate `card_id` on the board of one of the caller's OTHER subscribed rooms, -/// switch her current room there, and retry the claim once. +/// The room whose board holds `card_id` — the ONE place that answers *which +/// activity does this card belong to*. /// -/// Returns `None` when no subscribed room holds the card (the original -/// wrong-room refusal stands, verbatim), `Some(result)` when the card was found -/// and a room-switched claim was attempted — that result REPLACES the refusal, -/// so a genuine contention in the card's room still reports as contention. -/// Rooms are tried in subscription order; a card exists on exactly one board -/// (boards are per-room), so the first hit is the only hit. -async fn claim_following_card_room( - airc: &Arc, - card_id: WorkCardId, - ttl_ms: u64, -) -> Option> { +/// A card carries no room of its own (airc's `WorkCard` has no such field); +/// boards are PER-ROOM, so a card's activity IS the room whose board it appears +/// on. Two callers need that fact and used to derive it separately — badly: the +/// wrong-room claim retry inlined the scan, and the claim-fired solve dispatch +/// did not derive it at all and went roomless instead (#425, measured: 13,209 +/// roomless turns, 35% of one citizen's cognition, invisible to every room). +/// +/// Rooms are tried in subscription order and a card exists on exactly one board, +/// so the first hit is the only hit. A caller can only read boards of rooms it is +/// SUBSCRIBED to, so this widens no visibility — it only stops discarding what +/// the caller can already see. +async fn room_holding_card(airc: &Arc, card_id: WorkCardId) -> Option { let set = airc.subscription_set().await.ok()?; - let current = airc.current_room().await.ok(); for sub in set.all() { let room = sub.as_room(); - if current.as_ref().is_some_and(|c| c.channel == room.channel) { - continue; // the room that already refused - } let Ok(board) = airc.work_board_in(&room).await else { continue; }; - if board.snapshot().cards.iter().all(|c| c.card_id != card_id) { - continue; + if board.snapshot().cards.iter().any(|c| c.card_id == card_id) { + return Some(room); } - if airc.join(&room.name).await.is_err() { - continue; - } - crate::probe!( - class = "work.claim.followed_card_room", - card_id = %short8(card_id.as_uuid()), - room = %room.name, - "claim-by-id targeted a card outside the current room — switched to \ - the card's room and retried (accept-or-redirect, never refuse-and-instruct)" - ); - return Some( - airc.claim_work_card(ClaimWorkCard { card_id, ttl_ms }) - .await, - ); } None } +/// Locate `card_id`'s room, switch the caller's current room there, and retry the +/// claim once. +/// +/// Returns `None` when no subscribed room holds the card, or when the card's room +/// IS the current one (the room that already refused has nothing to follow) — in +/// both cases the original wrong-room refusal stands, verbatim. `Some(result)` +/// means a room-switched claim was attempted and that result REPLACES the refusal, +/// so a genuine contention in the card's room still reports as contention. +async fn claim_following_card_room( + airc: &Arc, + card_id: WorkCardId, + ttl_ms: u64, +) -> Option> { + let room = room_holding_card(airc, card_id).await?; + let current = airc.current_room().await.ok(); + if current.as_ref().is_some_and(|c| c.channel == room.channel) { + return None; + } + if airc.join(&room.name).await.is_err() { + return None; + } + crate::probe!( + class = "work.claim.followed_card_room", + card_id = %short8(card_id.as_uuid()), + room = %room.name, + "claim-by-id targeted a card outside the current room — switched to \ + the card's room and retried (accept-or-redirect, never refuse-and-instruct)" + ); + Some( + airc.claim_work_card(ClaimWorkCard { card_id, ttl_ms }) + .await, + ) +} + /// Claim a work card for this persona (as its own airc key). pub struct WorkClaim { pub registry: PersonaAircRuntimeRegistry, @@ -549,14 +566,41 @@ impl ActionCommand for WorkClaim { // nobody in the loop. Best-effort: a dispatch failure never voids the // claim — the claim is hers either way, and the probe says what happened. if let Some(caller) = ctx.caller.as_ref() { - // ROOMLESS, and named as such rather than papered over: the claim verb - // carries no activity — a citizen can claim from anywhere, and the card - // does not remember which room staged it. So a claim-fired solve still - // works invisibly. That is #425's whole subject (a bench claim must lead - // to IN-ROOM work, not a detached nil-room solve); the dispatch path - // below already has a room and passes it, so the two paths now differ - // exactly at the gap #425 exists to close. - dispatch_staged_swe_solve(ctx, &airc, caller.peer_id.as_uuid(), card_id, None).await; + // IN THE CARD'S OWN ROOM (#425). This used to pass `None` and say so — the + // claim verb carries no activity, so a claim-fired solve ran where nobody + // could see it: no act receipts (`apply_act` skips a nil room by design), no + // peer, no human, no ViewState. Measured before this fix: 13,209 roomless + // turns across the 25 newest trace files, 35% of one citizen's cognition. + // + // The room was never actually unknown — boards are PER-ROOM, so the card's + // activity is the room whose board holds it, and the wrong-room claim retry + // right above already resolves exactly that. It is now ONE resolver + // ([`room_holding_card`]) with two consumers instead of a scan here and a + // shrug there. A card we cannot place gets NO detached fallback: it says so + // on the probe and the claim still stands, because inventing invisible work + // is what this fix removes. + match room_holding_card(&airc, card_id).await { + Some(room) => { + dispatch_staged_swe_solve( + ctx, + &airc, + StagedSolveDispatch { + claimer: caller.peer_id, + card: card_id, + room: room.channel, + }, + ) + .await; + } + None => crate::probe!( + class = "work.claim.unplaceable_card", + card_id = %short8(card_id.as_uuid()), + claimer = %short8(caller.peer_id.as_uuid()), + "claimed a card no subscribed room's board holds — the claim STANDS, \ + but no solve fires: work whose activity we cannot name is work no \ + room can see (#425)" + ), + } } Ok(WorkClaimResult { card_id: p.card_id, @@ -569,6 +613,37 @@ impl ActionCommand for WorkClaim { /// This is the SWE adapter's N, not a global — each benchmark adapter owns its own. const SWE_CLAIM_ATTEMPTS: u32 = 3; +/// WHO works, WHICH card, WHERE the work is visible — the three facts that define one +/// staged-SWE dispatch, as a value object rather than a positional argument list. +/// +/// WHY A STRUCT (Joel, 2026-08-17: *"make sure params are well formed structs and +/// constants, good OOP, not random parameter lists"*). The previous signature was +/// `(ctx, airc, Uuid, WorkCardId, Option)`: two BARE UUIDs distinguished only by +/// argument POSITION, so transposing claimer and room compiled cleanly and dispatched a +/// citizen's solve under a room's id. Typed fields make that a compile error, and the next +/// fact this dispatch needs becomes a FIELD instead of a sixth argument. `ctx` and `airc` +/// stay separate parameters on purpose — they are ambient services, not facts about the +/// dispatch, and folding them in would turn the value object into a context bag. +/// +/// WHY `room` IS NOT `Option` (Joel, same day: *"if something is required, remove the +/// option… a required param is caught at compile time not runtime"*). It used to be +/// `Option`, and `work/claim` passed `None` — which is #425: the solve ran, her acts +/// radiated no receipts (`apply_act` skips a nil room by design), and no room, peer or human +/// ever saw the work. Making the field required moves that from a runtime shrug to an +/// unrepresentable state: a caller that cannot name the activity cannot build the struct, +/// so it must resolve one ([`room_holding_card`]) or report why it could not. +pub(crate) struct StagedSolveDispatch { + /// The citizen who does the work — her own airc identity. + pub claimer: crate::identity::PeerId, + /// The card she is working. + pub card: airc_work::WorkCardId, + /// The activity the run BELONGS to: `benchmark/dispatch` passes the per-run room it + /// just spawned (#329), `work/claim` resolves the card's own room. Every act the + /// solver executes radiates a receipt into it (#243), which is what makes the work + /// visible where the work lives. + pub room: airc_core::RoomId, +} + /// The #346 claim→solve dispatch. Fires a detached [`crate::commands::agent::solve::AgentSolve`] /// for the claimer when the claimed card matches a checkout `benchmark/swe-setup` staged /// into HER workspace: one staged instance directory whose name appears in the card @@ -584,17 +659,16 @@ const SWE_CLAIM_ATTEMPTS: u32 = 3; /// 2026-08-11: cards staged + assigned, zero claims, zero solves). The solve is her WHOLE /// cognition with an exclusive warm slot (`quiesce_others`), so nothing about "she does the /// work herself" changes — only the trigger moves off the chat turn. -/// `room` is the activity the run BELONGS to — `benchmark/dispatch` passes the -/// per-run room it just spawned (#329), and every act the solver executes then -/// radiates a receipt into it (#243). `None` is the roomless shape: her work -/// happens, and no room ever sees it. pub(crate) async fn dispatch_staged_swe_solve( ctx: &Ctx, airc: &std::sync::Arc, - claimer: uuid::Uuid, - card_id: airc_work::WorkCardId, - room: Option, + dispatch: StagedSolveDispatch, ) { + let StagedSolveDispatch { + claimer, + card: card_id, + room, + } = dispatch; let Ok(board) = airc .work_board_complete(airc_lib::WORK_BOARD_PROJECTION_PAGE_SIZE) .await @@ -679,7 +753,11 @@ pub(crate) async fn dispatch_staged_swe_solve( capture_dir: None, learn: crate::cognition::learning_policy::LearningPolicy::LearnFromThisWork, max_acts: None, - room, + // `AgentSolveParams::room` is still `Option` because `agent/solve` is also an + // operator-invocable command; THIS caller always has one, which is the point of + // [`StagedSolveDispatch::room`]. Narrowing the command's own param is the next + // slice, not a silent widening here. + room: Some(room.as_uuid()), path_prepend: Some(vec![venv_bin]), suppress_recall: None, prev_failed_patch_sha: None,