Mshearer/orchestration simplification [no merge] - #374
Draft
Shearerbeard wants to merge 54 commits into
Draft
Conversation
Sessions working on this branch recover program state from the process and board files in terminalbench-aura; record that pointer in CLAUDE.md so a cold session finds them without the spawning conversation. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
Land the DMMF type skeleton for the coordinator context redesign: 31 public types across eight files under orchestration/context/, with todo bodies behind parsing constructors per the type discipline in the program's PROCESS.md. The types encode the approved R1 decisions: pinned verbatim goal, evidence-framed task entries, 120-char failure handles, direct-plus-transitive worker frame under a token budget, and compact decision turns. Each public type maps to a business rule in the program repo's docs/redesign/TYPE_PLAN.md. Registering the module required dropping the dead prompt_constants context re-export from orchestration/mod.rs (E0255 name collision); its only consumer imports prompt_constants directly. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
ErrorPreview owns its truncation bound as an associated constant (2000, matching the default result_summary_length width today's renderer reuses for errors) instead of a caller-supplied width, for consistency with FailureHandle. FailureHandle docs pin the cap accounting: the truncation marker is appended after the 120-character cut, identically in display and grouping key. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
Rename Attestation to WorkerClaim: the type is the worker's own submit_result claim, and the old name carried no domain meaning. Field and variant names follow (claim, ArtifactStandIn::Claim), as do the error variant and messages. Rename WorkerContextFrame to PriorWorkFrame: the type models only the READ-ONLY PRIOR WORK block that fills the %%CONTEXT%% slot, not the worker's whole context, and the old name overclaimed. Add a TryFrom<&StructuredTaskOutput> stub for WorkerClaim, the one canonical unary conversion at the module boundary. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
Card R3a of the coordinator context redesign. The continuation prompt renders the verbatim original user query under a pinned goal line and presents completed, failed, and blocked tasks as worker-reported evidence with correlation labels only; coordinator task descriptions are no longer replayed into the continuation. Implements the R2 context-module bodies for goal, labels, evidence entries, and failure history; rewrites build_continuation_prompt onto the typed entries; wires the pinned goal at the post-execute IterationContext site while trigger_replan's carry-forward context stays unpinned. Failure handles cut at 120 chars with the marker appended after the cut, error previews own a 2000-char bound, and degrade renderings are mutually exclusive, each pinned by a unit test. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
Card R3b of the coordinator context redesign. The assistant turn recorded after a routing decision is now compact text on the typed CoordinatorTurn: create_plan records the variant, plan shape, and rationale with no task bodies; respond_directly and request_clarification record the model text verbatim. This removes the history side of the double render: the pretty-printed PlanningResponse JSON no longer enters the coordinator conversation or the planning-phase journal record, and a frame test pins that a task description appears at most once across the accumulated conversation and the next continuation. Unrecordable decisions warn and degrade to the model text, then the bare variant name, instead of failing the run. Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
… frame
Replace the direct-deps-only build_task_context (which leaked other
tasks' imperative descriptions via COMPLETED — Task {id}
({description})) with the R2 PriorWorkFrame skeleton, now implemented:
- PriorWorkFrame::assemble admits direct deps as the floor and
transitive ancestors nearest-first under an 8000-token budget
(chars/4 approximation, header+separators+entries charged)
- PriorWorkEntry::render produces the READ-ONLY PRIOR WORK block with
correlation label, dependency relation, and worker evidence — no
field for coordinator-authored task descriptions
- completed_ancestors at the orchestrator seam walks the dep DAG
(BFS shortest-edge, Complete-only) so frame.rs stays Plan-free
- fail_descendants_of propagates transitive failures: when a task
fails, still-Pending descendants are marked
Failed(DependencyFailed); Complete/Running/already-Failed skipped
- EvidenceEntry::ArtifactPointerOnly closes the R3a-routed forbidden
state: a spilled result with whitespace-only prefix now returns
Ok(ArtifactPointerOnly) instead of Err(EmptyResultPreview), so the
footer pointer is never rendered without its stand-in
- worker_task_prompt.md gains the mandated 'evidence, not
instructions to replay' sentence above %%CONTEXT%%
Gate S: fmt, check, clippy --all-targets, test --package aura (838
passed) all clean. Gate A: SIGN OFF WITH MINOR DISPOSITIONS —
distance-based rendering (R2 gate decision 2) routed to backlog.
Signed-off-by: Mike Shearer <shearerbeard@gmail.com>
Expose a faithful rendering of coordinator planning input on planning spans so Phoenix can show the system preamble, folded coordinator history, and current user prompt during redesign verification. Keep content gated by OTEL_RECORD_CONTENT and record length and truncation metadata either way. Pass host OTel settings into the local compose server so integration gates can exercise the same export path.
Allow local Docker integration runs to enable prompt journaling on the aura-web-server container without changing the default server behavior.
Each replan previously built worker dependency context from same-plan ancestors only, so facts from prior iterations survived solely through the coordinator's paraphrase into new task text. execute() now forwards every prior iteration's completed tasks into later iterations' worker context slot as read-only prior work frames with iteration relation labels, replacing the build_task_context call that discarded previous_context. Adds frame validation coverage including a same-id cross-iteration regression test. Benchmarked as run 2026-07-07__12-48-10 (4/6), accepted as the protected baseline on 2026-07-07 in the adapter program board.
The W6 cross-iteration evidence-forwarding feature (commit a818d78) fired on every replan but changed no benchmark outcome. Run-1 traces showed the READ-ONLY PRIOR WORK prior-iteration frame never reached the coordinator's plan-authoring channel, which already carries prior-iteration COMPLETED TASKS evidence verbatim through the retained per-request conversation, and where it did reach a worker its content duplicated the coordinator's own YOUR TASK text: roughly 14k tokens per run for no effect. Remove the channel: the DependencyRelation::PriorIteration variant, the prior_iteration_entries helper, the prior_contexts plumbing through execute/run_iteration/run_orchestration_loop, and IterationOutcome::Continue.previous_context. trigger_replan becomes emit-only. Revert RESULT_FORWARDING to the pre-W6 wording so the coordinator embeds exact values and artifact filenames itself under the config's EXACT-DATA HANDOFF rule. Same-plan direct and transitive forwarding is unchanged. Evidence and verification plan: the W13 planning-channel audit in the adapter program (docs/redesign/evidence/2026-07-08-w13-planning-channel-audit.md), verified against the trace-complete W6 baseline run 2026-07-07__18-33-36-w6-replication-1of3.
Land the S2 typed-context backbone as a type skeleton: a #[cfg(test)] context_fixture module whose scenario types compose the existing context-module parse-don't-validate types (PinnedGoal, EvidenceText, WorkerClaim, SpilledArtifact) and production state types (Plan, FailureSummary, ToolTraceEntry, RunManifest) into coordinator and worker call scenarios; a RequestEnvelope seam (system, messages, tools) whose builders will call the real assembly functions; and a location-anchored two-pass snapshot normalizer (offset-0 timestamp scrub, first-message worker-order canonicalization, behind a marker-occurrence audit that panics rather than rewrite payload bytes). All bodies are todo!() pending the implementation step - this commit is the schema the epic's envelope-identity claim builds on, repaired against the two-layer adversarial type review before any snapshot is baselined. The schema encodes the branch facts the review pinned down: the worker vector-store append is named-role-only (the Generic variant has no field for one), the planning budget derives from the roster config, session-history manifests are most-recent-first as load_session_manifests returns them, the populated worker frame derives its task text from its own plan task, and completed outcomes cannot name a worker absent from the roster. Skill tool definitions are included (SkillToolset::new is pure over SkillConfig); the remaining vector/scratchpad omissions are declared as partial-tools fixtures rather than folded into the reachability claim. Product files gain only three #[cfg(test)] delegating accessors in orchestrator.rs for the private continuation wrapper, worker prompt sections, and per-iteration failure fold; no production visibility widened, no behavior change. insta added to dev-dependencies for the snapshot corpus. The coverage manifest (MANIFEST.md) enumerates every preamble block, continuation slot, wrapper sub-slot, worker branch, and tool definition against a planned fixture or a named exclusion, and marks re-stated rules as re-stated rather than covered until the required comparison gates land; DESIGN.md records the type-to-rule inventory, the normalization spec, the net-reduction measurement contract, and residual risks R1-R8 (rig-fork mapping, timing, re-stated append orders, event side effects, trace-merge re-statement, MCP-sourced inventory content, escape-hatch pinning, conversation-growth and tool-order re-statement). Card: S2
… gates Fill every todo!() body in context_fixture/: parse-don't-validate fixture constructors, the envelope builders that call the real production assembly functions, and the location-anchored two-pass normalizer with its marker-occurrence audit. Land the snapshot corpus (18 fixtures, one per MANIFEST covered row) with committed .snap baselines, plus the REQUIRED R3/R5 comparison gates: the composed coordinator preamble byte-equals real create_coordinator output over a tempdir-backed skills+session-history config, and the in-memory trace merge equals load_tool_records_for_task over tempdir persistence. Two deviations from the skeleton, both recorded in DESIGN.md next commit: WorkerRosterFixture now also carries the agent-level vector-store catalog (Full-visibility tool descriptions read agent_config.vector_stores, a second production input the skeleton conflated with the coordinator append), and orchestrator.rs gains one more cfg(test) pure-delegation accessor (coordinator_preamble_for_golden) because the R3 gate cannot reach the private create_coordinator otherwise. The three skeleton accessors lose their expect(dead_code) attributes now that the corpus consumes them. Card: S2
Remove the 29 frame_validation_tests.rs cases whose asserted substrings the golden-frame snapshots now byte-lock, per the C8 measurement contract: only substring-subsumed cases go, and each deletion maps to the fixture that subsumes it (ledger in context_fixture/DESIGN.md). The 18 retained tests own coverage the corpus deliberately excludes: gated completed-task tool chains, degenerate inputs the fixture types forbid, the session-history catch-all render, multi-pattern failure ordering (HashMap-ordered, not snapshot-stable), the R3b at-most-once acceptance property, and fail_descendants_of plan machinery. Card: S2
MANIFEST.md: mark the corpus implemented, close the two RE-STATED rows the landed comparison gates now cover (coordinator preamble append order via R3, cross-iteration trace merge via R5), keep the worker-side append orders re-stated with the create_worker seam residue named, pin the corpus determinism constraints (single repeated failure pattern, single attempt per task per iteration), and name the owning tests on every exclusion row that leans on legacy unit coverage. DESIGN.md: record the two implementation-step deviations (roster vector catalog, fourth cfg(test) accessor), update R3/R5 to gate status with residues, add the C8 measured outcome with the 29-row deletion ledger and 18-row retained ledger, and add the verification record: byte-identity proof (no-op refactor plus negative control), slot-coverage proof via slot_coverage.sh (committed alongside), spill byte-layout confirmation, and the loc_measure classification caveat. Both records swept for the vale findings (em dashes, tricolons, AI-tell phrasings); vale reports 0 errors, 0 warnings. Card: S2
The slot-coverage populated-%%CONTEXT%% witness now uses the frame subtitle: the worker task template emits READ-ONLY PRIOR WORK unconditionally (defect B), so the old witness matched empty-frame snapshots and the check could never fail. Two new owning tests close ledger rows that cited coverage nobody owned: the compact-decision-turn fallback tiers (model text, then bare variant name) and the whitespace-only completed-result bare-label arm. The redundant second envelope construction and tools_json re-assert in the empty-frame identity test are gone (the normalized document already embeds all three surfaces), and the envelope module docs drop the stale gates-still-pending tense plus the nonexistent create_coordinator_agent symbol. Card: S2
The retained-test ledger now names a real owning test on every
exclusion row that leans on legacy coverage; two of those rows (the
compact-turn fallback tiers and the whitespace-only completed-result
bare-label arm) had no owner at all until the repair round added one.
The C8 measurement boundary now names its three pathspecs explicitly -
the displayed context_fixture{,.rs}/*.rs expression did not select the
facade the reported numbers included - and the re-measured delta at the
repaired head is 3,283 insertions / 892 deletions (net +2,391 test .rs
lines), recorded with an explicit Gate U deviation note: the card's
anticipated net reduction did not materialize. The loc_measure.py
caveat now covers both misclassifications (harness .rs as product,
design records as template). The re-stated-rules qualifier claims a
partial coordinator-side R3 closure instead of two full closures, the
RequestEnvelope invariant distinguishes structural completeness from
production-complete tools, R8 and the facade doc name the real symbols
and accessor count, and the byte-identity proofs are recorded as a
reproducible transcript (re-run this round: 27/27 clean on the no-op,
10 coordinator failures on the one-byte control).
Card: S2
The user's S2 schema review picked golden vocabulary for the harness test module. The file, the facade mod declaration, and the doc and path references move; the committed snapshots are untouched because their names derive from the normalize module path, not this one. Card: S2
Add a typed bounding module that consolidates every truncate, summarize, spill, display-limit, and history-limit decision behind one BoundingConfig derived from OrchestrationConfig. The module models the exact semantics production accepts today (zero thresholds, misordered duplicate-call thresholds, max_tools_per_worker=0) without tightening validation. The skeleton passed a two-model design panel (codex GPT-5.6 adversarial type review + Claude Opus 4.8 logic review) across two repair rounds: round 1 fixed 8 blocking findings (phantom states from strict validation, unit erasure, incomplete inventory); round 2 fixed 5 more (unreachable NudgeOnly variant, phantom Result returns from infallible construction, DuplicateCallPolicy fields made non-constructable via private newtypes, separate truncate_to_summary for the replan-error site, narrowed single-source-of-truth claim). Phase A only: method bodies implemented, no production call sites rewired yet. 22/22 golden-frame tests pass (envelope identity holds by construction). 842/842 lib tests pass. Card: S3
Add a private TruncateMarker primitive (None/EllipsisChar/Dots) with truncate_bytes and truncate_chars helpers, collapsing all 17 width-type truncate bodies to one-line calls. Char truncation uses char_indices().nth() (single-pass slice) matching the existing failure_history and evidence patterns. Replace pub const DEFAULT: usize with typed pub const DEFAULT: Self built via const NonZeroUsize::new, removing the private new() from every fixed width type. from_orchestration and default_widths now use Type::DEFAULT directly. Replace truncate_to_summary's (String, bool) return with TruncatedSummary implementing std::fmt::Display, giving .to_string() via blanket impl plus was_truncated() and a consuming into_string(). decide delegates to truncate_to_summary internally (DRY) and uses into_string to avoid the double allocation. Add a #[cfg(test)] module with 8 tests covering emoji/CJK boundaries, decide branches (inline/spill/threshold=0/summary>threshold), DuplicateCallPolicy collapse, SessionHistoryLimit states, marker styles and suppression, and TruncatedSummary Display. Update DESIGN.md: add TruncatedSummary and TruncateMarker to the type inventory, strip orchestrator.rs:NNNN line-number anchors, add residual risks R8 (SessionHistoryLimit compaction future), R9 (marker inconsistency), R10 (was_truncated signal asymmetry for char-cap types). Drop unused_variables from the module allow (dead_code stays for Phase A additivity). Card: S3
Add a BoundingConfig field to the Orchestrator struct, built from OrchestrationConfig at construction time. This is the S3 Phase B skeleton: the field is not yet read at any call site. Card: S3
Wire rows 2-6 and 10-11 of the bounding DESIGN.md seam table: - maybe_create_artifact uses ResultSpillBudget::decide/truncate_to_summary - PersistenceWrapper uses ToolOutputSpillBudget thresholds - format_tool_list and worker sections use ToolListLimit - DuplicateCallGuard uses DuplicateCallPolicy accessors - load_session_manifests uses SessionHistoryLimit - All log-preview safe_truncate sites use LogPreviewWidths - All plan-content truncate_query sites use PlanContentWidths - build_execution_summary uses truncate_to_summary for replan errors - Manifest result_preview/response_summary use ManifestWidths - Delete the now-unused truncate_query free function and its tests - Thread BoundingConfig through enforce_routing_config (no self) - Add SizePromotion::threshold_bytes and DurationPromotion::threshold_millis 22/22 golden frames green, 847/847 lib tests pass, clippy clean. Card: S3
Wire rows 7-9 of the bounding DESIGN.md seam table and fix R10: - Add truncate_with_flag to FailureHandleWidth and ErrorPreviewWidth (returns (String, bool) so constructors can gate their own markers) - Add failure_handle_width, error_preview_width, tool_reasoning_width fields to IterationContext, defaulted to DEFAULT - Add with_bounding_widths builder, set from self.bounding in orchestrator - FailureHandle::from_description now takes FailureHandleWidth - ErrorPreview::new now takes ErrorPreviewWidth - render_tool_chain_lines takes ToolReasoningWidth, delete truncate_reasoning - Add Serialize/Deserialize derives for serde compat on IterationContext - Update all test call sites to pass DEFAULT R10 choice: was_truncated-returning variant on char-cap types (option 1 from the kickoff), because the constructors own markers that differ from the TruncateMarker enum. 22/22 golden frames green, 847/847 lib tests pass, clippy clean. Card: S3
…unding Remove the blanket #![allow(dead_code)] from bounding.rs and add targeted #[allow(dead_code)] annotations to each unused API-surface item (15 items: constructors, convenience methods, and ScratchpadBudget which is row 12, not wired by design per R7). Card: S3
Workstream 2: retire the S2 harness's re-stated builders to real seams. R3 residue (b) - worker-side create_worker append order: - Always capture preamble in create_worker (remove prompt_journal gate) - Add worker_preamble_for_golden test accessor - Add gate_r3_worker_preamble_matches_create_worker gate - Residue: scratchpad append needs MCP, compared with NotWired R8 tool-registration-order: - Add coordinator_tool_order_for_golden accessor mirroring build_agent_with_tools - Add gate_r8_coordinator_tool_order gate - Add gate_r8_worker_tool_order shape assertion R8 conversation-growth: - Extract push_user_turn and push_assistant_turn from plan_with_routing - Add push_*_turn_for_golden test accessors - Update envelope.rs coordinator_envelope to use shared helpers - Add gate_r8_conversation_growth gate 26 golden tests (22 existing + 4 new), 851 total lib tests, clippy clean. Card: S3
- MANIFEST §4: conversation-growth rows flipped from RE-STATED to production-emitted (R8 closed by gate_r8_conversation_growth) - MANIFEST §5: worker-append-order rows flipped from RE-STATED to production-emitted (R3 closed by gate_r3_worker_preamble_matches_create_worker) - context_fixture DESIGN.md R3: worker-side comparison landed, scratchpad append is a conditional residue (needs MCP) - context_fixture DESIGN.md R8: all three seams landed and passing - bounding DESIGN.md R10: resolved, truncate_with_flag added Card: S3
Card: S3
Finding 1 (BLOCKING): DuplicateCallPolicy nudge_threshold unwrap_or(0) caused behavior change when nudge >= block. Fixed: unwrap_or(block) preserves block-only behavior since guard checks block first. Findings 2-3 (BLOCKING): R8 tool-order gates downgraded from "production-emitted" to "shape-asserted" (they mirror production order but do not call build_agent_with_tools directly). R8 conversation-growth downgraded to "partially production-emitted" (shared push helpers are production code, but sequence construction is test-side). Added generic-worker branch to R3 worker gate. Finding 4 (BLOCKING): Residual risk list on card completed with all open risks from both DESIGN.md files. Findings 5-7 (MINOR): LOC explanation added, stale design prose updated, Vale lint issues fixed. 26/26 golden tests, 851/851 lib tests, clippy clean, fmt clean. Card: S3
Gate D drift audit (board-owner session, 2026-07-15) sampled the living docs against card/S3 head 6f9a4ab and found four prose-only drifts, all inside this card's own diff: - D1: the MANIFEST re-stated-rules intro still said the worker append orders and R8 conversation-growth rows stay re-stated, contradicting the flipped rows in sections 4 and 5. - D2: the context_fixture DESIGN.md seam summary still claimed four cfg(test) accessors and no other product edits; phase B added four more accessors, one cfg(test) constructor, and two behavior-preserving production edits (create_worker unconditional preamble capture, the push_user_turn/push_assistant_turn extraction). - D3: the bounding DESIGN.md phase-b sentence made the same stale 'only accessors' claim. - D4: both docs said the R3 worker gate 'runs scratchpad-disabled'; the gate's config enables scratchpad but the MCP-less test environment leaves it unwired. Also fixes two vale findings in adjacent pre-existing prose (an em dash and a colon-tricolon false positive). No code changes. Card: S3
Delete the prompt_journal module (387 lines) and every reference to it in orchestrator.rs, mod.rs, and env_flags.rs. The journal was a failed diagnostics experiment, verbose, and never meant for OSS consumers. Gate A (opencode rust-reviewer, GLM-5.2) found three MINOR side effects of the removal, all fixed in this commit: - Orphaned current_iteration AtomicUsize field (sole reader was the removed journal_record); field, init, store sites, and AtomicUsize import removed - Stale AgentWithPreamble doc comment updated to reference the R3 golden-frame gate instead of journal recording - Dangling AURA_PROMPT_JOURNAL entry removed from compose/base.yml The golden-frame corpus passes byte-identically (841 tests, 0 failed), proving the request envelope is unchanged by the removal. DESIGN.md R3 residual notes updated to record the journal module's removal by card S24. Card: S24
Add persistence_lock spans around every ExecutionPersistence lock acquisition and persistence.* spans around each write operation. The tool_wrapper on_complete spawn now inherits the current span so persistence bookkeeping appears as a child of the tool execution span instead of being conflated with LLM streaming. No prompt-surface, frame-building, or persistence logic changed. Card: S23
Add timeout and worker_name to tracing::instrument skip lists to suppress redundant Debug fields (Findings 1, 2). Rename lock operation label from write_run_manifest to write_manifest for span name consistency (Finding 3). Card: S23
…ixtures Delete 5 brittle tests (3 from frame_validation_tests.rs, 2 inline in orchestrator.rs) whose assertions duplicate golden-frame coverage. Add 3 new golden-frame fixtures (coordinator_call_completed_task_tool_chain, coordinator_call_all_failure_categories, session_history_catch_all) that cover the surfaces the deleted tests targeted. Remap MANIFEST.md coverage table. Net test-LOC reduction of ~374 lines. Card: S17
Delete test_continuation_all_failure_categories (Finding 1, BLOCKING): the golden fixture coordinator_call_all_failure_categories covers all 10 FailureCategory variants vs the unit test's 8. Fix stale cross-reference in catch_all_manifest doc comment (Finding 4). Name the two previously unmentioned deleted tests in MANIFEST remap (Finding 2). Card: S17
Extend TemplateVars to cover all 9 prompt templates (was 2 of 9). Move build_planning_wrapper and workers-section builders from raw format! calls to template files with typed vars. New template files: planning_prompt.md, continuation_wrapper.md, worker_roster.md, worker_guidelines.md. Envelope identity preserved across all S2 manifest surfaces. Card: S4
Update stale doc reference in envelope.rs from config::WORKER_PREAMBLE_TEMPLATE to templates::WORKER_PREAMBLE_TEMPLATE (Finding 1, MINOR). Card: S4
Move artifact storage, spill trigger, pointer render, and the read tool from persistence.rs, orchestrator.rs, context/evidence.rs, and tools/read_artifact.rs into one owner module at persistence/artifacts/. persistence.rs stays as a facade; the read tool file re-exports from the owner module. Behavior on every S2 manifest surface is byte-identical (852 lib tests pass under INSTA_UPDATE=no). Card: S5
Restore four deleted manifest tests (test_manifest_serde_roundtrip, test_write_manifest, test_write_manifest_disabled, test_run_status_serde). Move allows_inline guard before lock_persistence to preserve original trace shape. Remove speculative Err fallback in spill.rs. Restore TOCTOU comment, # Errors rustdoc, and TrailingFooter domain doc. Reword module doc to describe what it is, not what it was. Card: S5
This was referenced Jul 17, 2026
…mpt work Delete chat_with_timeout (no callers), the unused prompt_constants section/field header module and its re-export, and eight fully-dead bounding.rs methods left after the S3-S5 consolidations. Deletion only: golden-frame envelope identity holds byte-for-byte, fmt and clippy -D warnings are clean, and the lib suite is green. The worker-order normalization sort stays (OrchestrationConfig::workers is a HashMap, iteration order still nondeterministic) and the documented-but-unwired ScratchpadBudget seam stays, both per bounding/DESIGN.md. Card: S6
…nces Gate A on S6 flagged two comments that still named the deleted chat_with_timeout method. Comment-only change; clippy and the lib suite stay green. Card: S6
Batch 1 of the S18 rolling ADRs (code decisions). Two MADR records matching the house docs/adr/ convention: the golden-frame context fixture schema from S2 and the unified bounding module from S3. Both are accepted decisions already implemented; the records trace every claim to the S2/S3 cards and the module DESIGN.md files. Card: S18
…imeout per_call_timeout_secs wraps the whole worker ReAct loop in stream_and_forward, so a flat wall-clock bound cannot distinguish a hung provider from a busy worker (#394). Add a detached stream_liveness module whose deadline re-arms on every stream item and fails the stream only after N consecutive silent seconds, with one thin seam in stream_and_forward's item loop. New config key inactivity_timeout_secs (default 0, disabled) lands beside per_call_timeout_secs, whose doc comment now says what it actually bounds for workers. Coordinator one-shot phases and provider-aware policy for silent GPT reasoning are out of scope. Fixes: #394 Card: S21
When persistence failed to write a spilled result artifact, maybe_spill_result returned the full unbounded result inline, silently defeating the spill threshold. Return the bounded summary with a visible failure marker instead. Fault-injection test forces the artifact write to fail and proves the inline result stays bounded and marked. Card: S14
Document the successful-spill return in maybe_spill_result's doc comment; drop the stale S14 candidate pointer from R6 now that S14 closed R2. Card: S14
The transient planning retry arm was a bare continue: it hot-looped against an overloaded provider and emitted nothing, so future occurrences stayed unmeasurable. Add a jittered exponential backoff (250ms base, 4s cap, 25% jitter) behind injected Sleeper/Jitter traits, and emit an orchestration.planning.retry span per attempt. Fault-injection tests prove delay bounds, exponential growth with cap, sleeper invocation, and span emission. Card: S22
- Sleep only when a retry actually follows (attempt < max_correction_attempts); the final attempt emitted a wasted backoff before the failure return. - Hard-clamp the final jittered delay to the cap instead of only clamping the unjittered base. - Reword production doc comments to describe behavior, not crates. Card: S22
Preview summaries sliced with &s[..200], which panics when byte 200 falls inside a multi-byte character (observed live on box-drawing output). Reuse string_utils::safe_truncate at all five sites; ASCII previews render byte-identical, asserted by tests. Card: S55
Gate A round-1 minor: the simpler preview pattern used by mcp_response.rs had no direct exact-string assertion. Card: S55
Card and process references (S2/S3/S17, R2/R3a/R3b/R3c gate decisions, skeleton) in comments are replaced with self-contained wording a cold mainline reviewer can read without tracker context. Residual-risk codes (R3/R5/R8) become short descriptions or MANIFEST.md section cites, and the _for_golden test accessors are renamed to *_for_test with doc comments describing what they expose rather than which card added them. Comment- and test-identifier-only; no behavior change, all 870 lib tests green with zero pending snapshots. Card: S28
Gate a findings: three type-level comments narrated behavior owned by implementing functions. Each now states only what the value is; the behavior was already documented at the implementing sites. Card: S28
2 tasks
The success arm of maybe_spill_result expected a non-empty filename
from write_result_artifact, but disabled persistence (the default
when no memory_dir is configured) returns Ok("") - so any
default-config run with a result over the 4000-char spill threshold
panicked on a supported configuration. The arm now matches the S14
err-arm shape: an unavailable artifact returns the bounded summary
with the visible write-failed marker. The DESIGN.md R2 note is
updated to describe both unavailability routes.
Card: S50
Gate a minors: bind and log the actual SpilledArtifact error instead of a wildcard with a hardcoded cause; document the disabled-persistence route on maybe_spill_result and the marker constant; assert the disabled path renders no artifact-pointer footer. Card: S50
When write_tool_output_artifact fails, the persistence wrapper stored the full clean tool output in the tool-call record - the same unbounded-on-error shape S14 fixed in maybe_spill_result. The record now holds a threshold-bounded summary plus the visible write-failed marker. The marker text avoids the trailing-footer parser prefixes, verified against parse_trailing and the artifact footer strip. A fault-injection test blocks the artifact write and asserts the bounded record. Card: S51
Gate a minor: the marker constant comment narrated behavior on a value definition; it now states only what the value is. Card: S51
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Experimental Fable agent fleet implementation of the simplification and continuation prompt restructuring I'm envisioning for benchmark shortcomings with context continuation with AURA. This also covers a very markdown friendly "frame" development development scheme. Each "frame" of a context turn has its own struct, constructor, and visual confirmation tests that make it easy for both humans and agents to see the exact outcome in the test and code to meet that outcome. This also adds replay-ability of context frames to understand exactly how a specific situation would render to the agent which should aid in retros or understanding how orchestration context gets passed.