Full spec scaffold (requirements/design/research/tasks all approved, ready_for_implementation=true) plus a roadmap backlog item recorded from the dirty-content-fingerprint spec's task 4.3 findings (Makefile/meson fingerprint-computation duplication risk). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
13 KiB
13 KiB
Research & Design Decisions
Summary
- Feature:
scheduler-sim-repl-verbosity - Discovery Scope: Extension (existing system) — light discovery
- Key Findings:
- Two independent, unconditional stdout print paths cause
continue-mode volume to matchstep-mode volume:xce_repl_on_event()(repl_break.c:708-731, gated only bysim->interactive, not byrs->mode) andsim_print_decision()(sim_loop.c:388-411, called directly fromsched_sim_driver_run()'s main loop with nosim->interactive/mode gate at all). xce_repl_post_session_check()(repl.c), called once per completed round fromsched_sim_driver_run()right after each round's decisions are drained, is the correct integration point for round-boundary rollup/collapse logic — it already runs exactly once per round when interactive, and already exists purely for thestep-budget pause check.xce_repl_on_decision()(repl_break.c:776-799) already tallieslast_n_dispatch/last_n_pend/last_n_preemptper round, but that tally is decision-event-driven — real replay data shows it may never observe a freshDISPATCHdecision in replay mode even when jobs demonstrably transition to running (bug-1611). The pend-jobid set for the round rollup must therefore be derived by directly queryingcore->jobsforJOB_IS_PENDstate (the same iteration idiomrepl_info_jobs()already uses), not solely from the decision tally, to satisfy Requirement 2.4 regardless of bug-1611's outcome.- Existing test file
test_repl_execution_control.calready drivessched_sim_driver_run()end-to-end via a child-process harness (child_continue_then_inspect,child_continue_twice_after_complete) that captures real process output — this is the correct home for behavioral tests of Requirements 1-3 (mode gating, rollup, collapsing), since it observes actual printed text, not just internal state. - No new external dependency is needed — all formatting is plain C string/stdio work consistent with the rest of
repl.c/repl_break.c.
- Two independent, unconditional stdout print paths cause
Research Log
Extension point analysis: where does continue-mode output actually originate?
- Context: Requirements 1-3 need continue mode to suppress/summarize output that today prints unconditionally.
- Sources Consulted:
repl_break.c(xce_repl_on_event,xce_repl_on_decision),sim_loop.c(sim_print_decision),repl.c(xce_repl_post_session_check,repl_should_pause),repl.h(interactive-gated inline wrappers),repl_internal.h(xce_sched_repl_state_s), CodeGraph call-graph forsched_sim_driver_run. - Findings:
xce_repl_check_event()(repl.h,static inline) gatesxce_repl_on_event()onsim->interactiveonly — insidexce_repl_on_event(),repl_print_event_summary()fires unconditionally before any breakpoint check, with no reference tors->mode.sim_print_decision()has no interactive or mode gate at all;sched_sim_driver_run()calls it directly for every drained decision, the same call site that also drivesxce_repl_check_decision()(which does the tally) andsched_record_session_add_decision()(recording).xce_repl_post_session_check()fires once per completed round, strictly after that round's decisions are drained and before the next round's events begin — confirmed via call-graph (sched_sim_driver_run → xce_repl_post_session_pause).
- Implications: Requirement 1's gate belongs inside
xce_repl_on_event()/repl_print_event_summary(). Requirement 2/3's rollup and collapsing logic belongs at thexce_repl_post_session_check()boundary, reading state thatxce_repl_on_decision()'s existing tally and a directcore->jobsquery both feed. Requirement 2/3 also requires suppressingsim_print_decision()'s direct per-decision print whensim->interactive && rs->mode == XCE_REPL_MODE_CONTINUE, since that print path is the one bug-1610 identified as unconditional.
Job identifier range compression: existing formatting conventions
- Context: Requirement 4 needs contiguous array-job indices displayed as ranges.
- Sources Consulted:
lib/utils.c(fmt_jobid/fmt_jobid_ex),repl_break.c'srepl_print_event_summary()(currentJOB_PUSHjobid list rendering). - Findings:
fmt_jobid_ex()already separates the packed 64-bit jobid into(cluster, apiserver, timestamp, job[, idx])components viaEXTRACT_*macros; an array job'sidxis already isolated during formatting. There is no existing range-compression helper anywhere in the REPL formatting code — every current jobid list (theJOB_PUSHline, and any future round-rollup pend list) is onefmt_jobid()call per id. - Implications: A new helper is needed; the natural base for grouping is the already-decoded
(cluster, apiserver, timestamp, job)tuple (identical for every element of one array submission), with onlyidxvarying. No change tofmt_jobid/fmt_jobid_exthemselves — the new helper wraps a list of ids, not a single id.
Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|---|---|---|---|---|
| Gate at existing hook boundaries (chosen) | Add mode/collapse logic inside xce_repl_on_event, sim_print_decision's call site, and xce_repl_post_session_check |
Reuses every existing per-round/per-event integration point; no new hook call sites needed (sim_loop.c already at its documented line-budget ceiling per repl.h's own header comment) | Requires touching two files (repl_break.c, sim_loop.c) instead of one |
Matches this codebase's own precedent (repl.h's inline-wrapper gating) |
| New dedicated "output policy" module | Introduce a separate repl_output.c owning all print decisions, called from a single new hook |
Cleaner separation of concerns | Adds a 6th call site into sim_loop.c's round loop, which repl.h's own header comment says there is no line budget for; larger diff for a formatting-only feature |
Rejected — over-engineered for the boundary this spec owns |
Design Decisions
Decision: Round rollup's pend-jobid set is state-derived, not decision-tally-derived
- Context: Requirement 2.4 requires the round summary to reflect real job status changes even though bug-1611 shows the decision tally may never register a fresh
DISPATCHin replay mode. - Alternatives Considered:
- Trust
xce_repl_on_decision()'s existinglast_n_dispatch/last_n_pend/last_n_preempttally directly for the pend-jobid set — simplest, but inherits bug-1611's blind spot if it turns out to be a real gap rather than benign replay semantics. - Derive the pend-jobid set each round by directly iterating
core->jobsforJOB_IS_PENDstate (same idiom asrepl_info_jobs()), independent of the decision tally.
- Trust
- Selected Approach: Option 2 for the pend-jobid set; the existing tally is still used for the dispatch/preempt counts shown in the rollup header, since bug-1611 only questions dispatch visibility and preempt is unaffected.
- Rationale: Requirement 2.4 must hold regardless of how bug-1611 resolves. A state query is authoritative by construction; a decision-tally is only as complete as whatever decisions the replay happens to (re-)generate.
- Trade-offs: One extra
core->jobsiteration per round in continue mode (bounded by job count, not a hot path — replay/step already tolerates equivalent iteration cost inrepl_info_jobs()/repl_count_jobs()). If bug-1611 is later confirmed a real gap, the dispatch count shown in the rollup may still under-report until that separate bug is fixed — flagged as a known limitation, not silently masked. - Follow-up: bug-1611 should be investigated (separately from this spec) before or during implementation; if it proves to be a real visibility gap rather than benign replay semantics, a follow-up task should switch the dispatch count to the same state-diff approach used for the pend set.
Decision: Collapse fingerprint keys on (dispatch_count, pend_jobid_set, preempt_count), not raw decision objects
- Context: Requirement 3.2 needs to detect when consecutive rounds are "the same" to collapse them.
- Alternatives Considered:
- Compare full decision-object lists round to round.
- Compare a compact fingerprint: dispatch count + preempt count + the state-derived pend-jobid set (as an ordered/canonical list).
- Selected Approach: Option 2.
- Rationale: The real replay validated this exact fingerprint shape — 65 consecutive rounds shared an identical
(0, {8 jobids from base A + 6 jobids from base B}, 0)fingerprint and collapsed correctly by hand-simulation. Comparing raw decision objects would be more expensive and no more precise, since the pend-jobid set already captures every observable difference the operator cares about. - Trade-offs: A change in why a job is pending (a future pend-reason feature under
scheduler-sim-diagnostics) would not by itself break the collapse, since the fingerprint doesn't include pend reason. Acceptable for this spec's boundary — pend-reason display is explicitly out of scope here. - Follow-up: If
scheduler-sim-diagnosticslater adds pend-reason display, revisit whether the fingerprint needs to include it (a revalidation trigger, see design.md Boundary Commitments).
Decision: Round Rollup exposes a shared flush entry point instead of owning every flush trigger itself
- Context: An independent task-graph sanity review (run before task generation) found that breakpoint-hit detection and structural-event classification both live in
repl_break.c(xce_repl_on_event,xce_repl_on_begin_round,xce_repl_on_decision, each callingrepl_command_loop()directly on a match) — not inrepl.cwhere the initial design placed all Round Rollup logic. Requirements 3.5 and 3.6 cannot be satisfied by round-boundary logic alone. - Alternatives Considered:
- Duplicate collapse-flush logic inside
repl_break.cas well asrepl.c— rejected, violates single-ownership of the collapse state and risks divergent flush behavior. - Move all breakpoint/structural-event detection into
repl.cso Round Rollup owns every trigger directly — rejected, a much larger and riskier change to existing, already-tested breakpoint-matching code with no benefit to this spec's boundary. - Round Rollup exposes one flush entry point (
repl_round_collapse_flush);repl_break.c's existing hooks call it before their existing structural-event print / breakpoint-triggered command-loop entry.
- Duplicate collapse-flush logic inside
- Selected Approach: Option 3.
- Rationale: Keeps collapse-state ownership entirely inside
repl.c(single writer, no duplicated logic) while letting the two other legitimate trigger sites (repl_break.c's breakpoint/structural-event detection, andrepl.c's ownxce_repl_on_replay_complete) call a small, idempotent, safe-on-NULL interface — consistent with this codebase's existing "safe no-op on NULL/not-yet-allocated state" convention already documented on every other REPL hook. - Trade-offs: Event Echo Gate (repl_break.c) now has a real outbound dependency on Round Rollup's interface, removing the parallel-safety the initial task draft assumed between those two components' implementation work — tests for both can still be written independently, but implementation must be sequenced.
- Follow-up: none — resolved before task generation;
design.mdand the task graph both reflect this.
Risks & Mitigations
- bug-1611 (dispatch visibility in replay mode) is unresolved — Mitigation: pend-set correctness (Requirement 2.4) does not depend on its resolution (see Design Decision above); dispatch-count accuracy is called out as a known, separately-tracked limitation rather than silently assumed correct.
sim_print_decision()'s exact call site insidesched_sim_driver_run()was confirmed via call-graph but its surrounding loop body was not fully read — Mitigation: implementation task must re-confirm the precise insertion point and existing control flow around it before editing; this is a normal implementation-time verification, not a design ambiguity (the gating contract — suppress undersim->interactive && continue mode— is unambiguous regardless of exact line placement).- No existing test coverage on
xce_repl_on_event/sim_print_decisiontext output (per CodeGraph) — Mitigation: Requirement-driven tests added intest_repl_execution_control.c(child-process output capture) before implementation, per this repo's TDD mandate.
References
.wolf/buglog.jsonbug-1609, bug-1610, bug-1611 — the three findings that motivated this spec, each with file:line root causes already recorded..kiro/specs/scheduler-sim-diagnostics/brief.md— sibling brief; Requirement 2.4's "why did it pend" is explicitly out of this spec's boundary and belongs to that brief's seed capability 2 if formalized later.