lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(scheduler-sim-eager-reconciliation): task 1.1 state-independent dispatch capture

Browse source 
Widens sim_reconcile_unaccounted_jobs()'s trigger gate in sim_loop.c: removes
!JOB_IS_RUN(job->state) so a job's recorded DISPATCH decision is captured as
soon as the replay reaches the round it occurred in, regardless of the job's
current state -- closing bug-1611/bug-1613's residual (a job whose RUN
confirmation lands in a later round than its dispatch decision, the realistic
case under any real dispatch-to-confirmation latency, was never reconciled).
sched_account_jobs()'s own unmodified JOB_IS_RUN charging gate continues to
defer actual resource-in-use accounting until the job genuinely transitions
to RUN -- no other line in the match/capture/report path changes.

Replaces test_reconcile_skips_non_run_state_job (which asserted the bug as
correct behavior) with test_reconcile_captures_non_run_state_job_when_dispatch_recorded,
proving the opposite for a PEND-state job. Adds the xce-test-rc-file-style
spec docs (requirements/design/research/tasks) for this feature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
Rongsong Shen 2026-08-27 03:45:08 +00:00
parent 3891bd909a
commit e2bb98c87e
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
6 changed files with 419 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

212
.kiro/specs/scheduler-sim-eager-reconciliation/design.md Normal file
View file

@ -0,0 +1,212 @@
# Design Document
## Overview
**Purpose**: This feature closes the residual gap in `scheduler-sim-decision-parity`'s Recorded-Decision Reconciliation mechanism (`sim_reconcile_unaccounted_jobs()`, `sim_loop.c`) so a simulation-mode replay correctly captures a job's resource allocation regardless of how many rounds separate its recorded DISPATCH decision from its RUN-state confirmation — the realistic case in any real deployment, and the dominant real-world manifestation of bug-1611/bug-1613.
**Users**: Scheduler operators debugging or tuning via `xce_scheduler -i -s <replay>` (interactive) or batch `--sim` replay.
**Impact**: Changes when `sim_reconcile_unaccounted_jobs()` is willing to search a round's recorded decision stream for a job — from "only if the job is already RUN this round" to "whenever the job has no captured allocation yet, regardless of its current state." No other function changes.
### Goals
- Capture a job's recorded DISPATCH allocation at the round it actually occurred in the original recording, independent of the job's state at that moment.
- Preserve correct, state-gated resource-in-use accounting: a captured-but-not-yet-running job still contributes zero to `resInUse` until it genuinely transitions to RUN.
- Make the dispatch decision observable (printed, recorded, REPL-checked) at the historically accurate round.
### Non-Goals
- Any change to `job->state` transition timing or triggers.
- Any change to simulation mode's live picker (still PEND-only for freshly-arriving jobs, per `scheduler-allocation`).
- Any change to `--sim-verify`'s comparison semantics beyond not introducing a new false mismatch.
- Reopening or modifying `scheduler-sim-decision-parity`'s own requirements, design, or tasks, or any normal-mode (`driver.c`) behavior.
## Boundary Commitments
### This Spec Owns
- The trigger condition of `sim_reconcile_unaccounted_jobs()` (`sim_loop.c`) — which jobs are eligible to be searched for a matching recorded DISPATCH decision each round.
- The test coverage proving capture now works across round separation, still respects the existing no-fabrication/no-double-capture guarantees, and does not regress `--sim-verify` or interactive/batch parity.
### Out of Boundary
- `sched_account_jobs()`'s own `JOB_IS_RUN` charging gate (`scheduler.c:1258`) — unchanged; it is exactly the correct deferral mechanism this feature relies on, not something it modifies.
- The match/capture mechanics once a job is eligible (jobid matching, ownership transfer of `xce_job_alloc_t`, `make_job_account()`, shift-down compaction, decision reporting via `make_dispatch_decision()`/`sim_print_decision()`/`xce_repl_check_decision()`) — all unchanged, all already correct per `scheduler-sim-decision-parity`.
- Normal-mode (`driver.c`) behavior, simulation mode's live picker, `--sim-verify`'s comparison algorithm itself, and any `job->state` transition logic.
### Allowed Dependencies
- `sim->core.jobs` iteration and `decoded_session->decisions` (both already owned/populated by `sim_reconcile_unaccounted_jobs()`'s existing implementation) — P0.
- `sched_account_jobs()`'s existing `JOB_IS_RUN` gate as the sole mechanism deferring actual resource charging — P0, must remain unmodified for this design to be correct.
- `make_job_account()`/`make_dispatch_decision()` (existing, unmodified) — P0.
### Revalidation Triggers
- Any future change to `sched_account_jobs()`'s charging gate (e.g. adding a new precondition beyond `JOB_IS_RUN`) must re-verify this feature's Requirement 1.2/1.3 still hold, since this design relies on that gate — not a new one of its own — for correct deferral.
- Any future change that makes simulation mode's live picker emit a DISPATCH decision (currently PEND-only, `scheduler-allocation`'s stated scope) must re-verify Requirement 3.3's dedup assumption, which currently holds structurally (no live DISPATCH ever competes with a reconciled capture) rather than via an explicit check in this feature's own code.
- Any change to `decoded_session`'s per-round decode/free lifecycle (`sim_loop.c:871`/`:1006`) must re-verify this feature's core premise (every round's own decisions are only ever visible during that round).
## Architecture
### Existing Architecture Analysis
`sim_reconcile_unaccounted_jobs()` runs unconditionally once per simulation round, strictly between `xce_core_begin_session()` (which zeroes `resInUse`/`sharedInUse`) and `sched_account_jobs()` (which re-sums them from every `JOB_IS_RUN` job with `job->accounts != NULL`) — see `sim_loop.c:912-957`. Its current trigger gate additionally requires `JOB_IS_RUN(job->state)` before considering a job at all:
```c
if ((job == NULL) || !JOB_IS_RUN(job->state) || (job->accounts != NULL)) {
continue;
}
```
`decoded_session` (this round's decoded recorded-decision stream) is freed at the end of the same round (`sim_loop.c:1006`) and never persisted — so a job skipped at round N because it was still PEND has no later opportunity to be matched against round N's data, even after it reaches RUN at round N+k.
### Architecture Pattern & Boundary Map
```mermaid
flowchart TD
RoundStart[Round begins] --> Decode[decode_ReplaySession this round]
Decode --> Begin[xce_core_begin_session resets resInUse]
Begin --> Reconcile[sim_reconcile_unaccounted_jobs]
Reconcile --> Gate{job accounts is NULL}
Gate -- no --> NextJob[skip already-captured job]
Gate -- yes --> Search[search this rounds decoded decisions for jobid]
Search --> Found{matching DISPATCH found}
Found -- yes --> Capture[capture allocs and accounts, report decision]
Found -- no --> Retry[leave accounts NULL, retry next round]
Capture --> Charge[sched_account_jobs]
Retry --> Charge
NextJob --> Charge
Charge --> ChargeGate{job state is RUN and accounts set}
ChargeGate -- yes --> ResInUse[include in resInUse]
ChargeGate -- no --> Defer[exclude from resInUse]
```
**Architecture Integration**:
- Selected pattern: single-condition removal from an existing, already-correct-in-every-other-respect mechanism — not a new component.
- Domain/feature boundaries: capture (this feature, `Gate`/`Search`/`Capture`/`Retry` above) stays fully decoupled from charging (`sched_account_jobs`, `ChargeGate` above, untouched) — the same separation normal mode already has between `sched_commit_decision()` (capture) and `sched_account_jobs()` (charge).
- Existing patterns preserved: per-round re-invocation, jobid-scoped matching, ownership-transfer capture, shift-down compaction, three-call decision reporting — all untouched.
- New components rationale: none — this is a boundary-widening change to one existing condition.
- Steering compliance: directly closes the gap `.kiro/steering/scheduler-driver-equivalence.md` documents as bug-1611's residual; testing discipline (contention fixture) from that same steering doc drives the Testing Strategy below.
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|-----------------|-------|
| Scheduler core (C, gnu17) | Existing `sim_loop.c`/`scheduler.c` | Trigger-condition change only | No new dependency; no schema/wire-format change |
## File Structure Plan
### Modified Files
- `src/scheduler/src/sim_loop.c` — remove `!JOB_IS_RUN(job->state)` from `sim_reconcile_unaccounted_jobs()`'s trigger condition (`sim_loop.c:592`); update the function's header doc comment (`sim_loop.c:~505-565`) to describe the new state-independent trigger and reframe the "double-application guard" description to also cover "this is now the sole capture-eligibility gate."
- `src/scheduler/testsuite/test_sim_reconcile_unaccounted.c` — replace `test_reconcile_skips_non_run_state_job` (asserts the bug: a PEND job with a matching recorded entry must stay uncaptured) with an inverted-expectation test proving a PEND job **is** now captured; add new tests for cross-round separation, `job->state` transitioning after capture, the no-recorded-origin-ever case, and the "captured-but-not-RUN contributes zero to resInUse" case (Requirement 1.2) driven through `sched_account_jobs()`.
- `src/scheduler/testsuite/test_sim_contention_parity.c` and/or a new contention-focused test file — add a resource-contention fixture (capacity insufficient for all candidate jobs) exercising cross-round dispatch-to-confirmation separation, per `scheduler-driver-equivalence.md`'s explicit testing discipline that a no-contention fixture cannot prove accounting correctness.
- A `--sim-verify` regression test (existing `test_sim_*` verify-mode file, or a new one if none fits) spanning a multi-round-separated dispatch — proves Requirement 4.2.
### No New Files
This feature introduces no new production source file — see Simplification decision in `research.md`.
## System Flows
```mermaid
sequenceDiagram
participant Replay as Replay stream
participant Recon as sim_reconcile_unaccounted_jobs
participant Job as job in core jobs
participant Acct as sched_account_jobs
Replay->>Recon: round N decoded decisions include DISPATCH for job X
Note over Job: job X state is PEND at round N
Recon->>Job: capture allocs and accounts from DISPATCH
Recon->>Recon: report decision at round N
Acct->>Job: check state is RUN
Note over Acct: state is PEND, skip charging
Replay->>Job: round N+k replays job X state change to RUN
Recon->>Job: accounts already set, skip re-search
Acct->>Job: check state is RUN and accounts set
Acct->>Acct: charge resInUse from round N+k onward
```
**Key decisions not obvious from the diagram**: the decision is reported once, at round N (the historically accurate round, Requirement 2.1) — not at round N+k when charging actually begins. Charging timing (round N+k) and reporting timing (round N) are intentionally decoupled, mirroring how normal mode's own `job->state` display and `resInUse` accounting are already decoupled from `sched_commit_decision()`'s synchronous capture.
## Requirements Traceability
| Requirement | Summary | Components | Interfaces | Flows |
|-------------|---------|------------|------------|-------|
| 1.1 | Capture at originating round regardless of state | `sim_reconcile_unaccounted_jobs` | trigger gate | Search/Capture |
| 1.2 | Captured-but-not-RUN excludes from resInUse | `sched_account_jobs` (unchanged, relied upon) | `JOB_IS_RUN` charge gate | ChargeGate/Defer |
| 1.3 | RUN transition includes in resInUse from that point | `sched_account_jobs` (unchanged, relied upon) | `JOB_IS_RUN` charge gate | ChargeGate/ResInUse |
| 1.4 | Matches normal-mode's equivalent-point reporting | `sched_account_jobs` (shared with normal mode) | n/a | n/a |
| 2.1 | Decision visible at historically accurate round | `sim_reconcile_unaccounted_jobs` (decision reporting, unchanged) | `make_dispatch_decision`/`sim_print_decision`/`xce_repl_check_decision` | Capture step |
| 2.2 | Original decision type/detail preserved | `make_dispatch_decision` (unchanged) | n/a | Capture step |
| 2.3 | No duplicate report when live decision also exists | live picker (unchanged, PEND-only — structural invariant) | n/a | n/a |
| 3.1 | Never capture from a non-originating decision | jobid match (unchanged) | `entry->body.dispatch->alloc->jobid` check | Search |
| 3.2 | Never re-capture once captured | `job->accounts != NULL` guard (unchanged, now the sole gate) | trigger gate | Gate |
| 3.3 | Already-captured job not overwritten by a later live decision | structural (live picker never emits DISPATCH) | n/a | n/a |
| 4.1 | Same-round case still works | trigger gate (RUN is a subset of "any state") | trigger gate | Gate/Search |
| 4.2 | No new `--sim-verify` mismatch | compaction ordering (unchanged), verified by new test | decoded_session compaction | Capture step |
| 4.3 | Interactive/batch parity preserved | shared `sched_sim_driver_run()` call site (unchanged) | n/a | n/a |
| 4.4 | No recorded origin anywhere: report unavailable, keep going | "not found" branch (unchanged) | TRACE log, `job->accounts` stays NULL | Retry |
| 4.5 | Normal mode unaffected | `driver.c` untouched | n/a | n/a |
## Components and Interfaces
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|-----------|--------------|--------|---------------|---------------------------|-----------|
| `sim_reconcile_unaccounted_jobs` | Scheduler sim driver | Widen capture eligibility to any not-yet-captured job, regardless of state | 1.1, 2.1, 3.1, 3.2, 4.1, 4.4 | `sched_account_jobs` (P0, relied on unmodified) | Service |
### Scheduler sim driver
#### `sim_reconcile_unaccounted_jobs`
| Field | Detail |
|-------|--------|
| Intent | Capture a job's recorded DISPATCH allocation as soon as the replay reaches the round it occurred in, independent of the job's current state |
| Requirements | 1.1, 2.1, 3.1, 3.2, 4.1, 4.4 |
**Responsibilities & Constraints**
- Owns only the capture-eligibility decision (which jobs are searched each round) and the search/capture/report mechanics already delivered by `scheduler-sim-decision-parity` (unchanged).
- Has no knowledge of resource charging — that remains `sched_account_jobs()`'s exclusive responsibility.
- Never fabricates data: a job with no matching recorded decision in a given round is left uncaptured and re-checked the following round (Requirement 4.4).
**Dependencies**
- Inbound: `sched_sim_driver_run()`'s round loop — calls this once per round, unchanged call site and unchanged position relative to `xce_core_begin_session()`/`sched_account_jobs()`/`xce_core_signal_go()` (P0).
- Outbound: `sched_account_jobs()` — this feature supplies `job->accounts` earlier than before; `sched_account_jobs()`'s own unmodified `JOB_IS_RUN` gate determines when that data starts being charged (P0).
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
##### Service Interface
```c
/* Unchanged signature. Behavior change: a job is now eligible for capture
* whenever job->accounts == NULL, regardless of job->state (previously
* additionally required JOB_IS_RUN(job->state)). Capturing a job's
* allocation while it is not yet RUN is inert until sched_account_jobs()'s
* own JOB_IS_RUN gate later includes it -- this function performs no
* charging itself. */
static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
replay_session_t* decoded_session,
sched_record_session_ctx_t* record_ctx);
```
- Preconditions: unchanged — `sim`/`decoded_session` NULL is a safe no-op; `record_ctx` may legitimately be NULL.
- Postconditions: unchanged — a matched job has `job->allocs`/`job->accounts` set and its entry removed from `decoded_session->decisions`; an unmatched job is left exactly as it was.
- Invariants (new): a job is captured **at most once**, at the first round (in replay order) whose decoded stream contains its originating DISPATCH decision — independent of that round's job state.
**Implementation Notes**
- Integration: delete `!JOB_IS_RUN(job->state)` from the trigger condition at `sim_loop.c:592`; no other line in the match/capture/report path changes.
- Validation: see Testing Strategy — contention fixture, cross-round separation, `--sim-verify`, interactive/batch parity, and the inverted `test_reconcile_skips_non_run_state_job` replacement are all required, not optional.
- Risks: repeated per-round "not found" TRACE logging for a job with no recorded origin anywhere in the replay (Requirement 4.4 case) — accepted, TRACE-level, see `research.md`'s Risks section.
- **Verified safe (early capture vs. cleanup)**: capturing `job->accounts` while a job is still PEND is safe against every existing job-cleanup path. `sched_uncharge_account()` has zero production callers anywhere in the scheduler — production resource accounting is a reset-and-resum model (`xce_core_begin_session()` zeroes `resInUse`/`sharedInUse` every round; `sched_account_jobs()` re-sums it from scratch from currently-`JOB_IS_RUN` jobs only). Both places `job->accounts` is freed on a non-RUN transition (`scheduler.c:974-977`, `core_events.c:915-918`) release memory only, with no `resInUse` mutation. A job captured while PEND that later transitions directly to a terminal state without ever passing through RUN is therefore never summed by `sched_account_jobs()` at any round and its accounts are freed with zero accounting impact — early capture cannot under-count `resInUse`. See `research.md`'s corresponding Research Log entry for the full trace.
## Error Handling
### Error Strategy
- **No matching recorded decision this round**: not an error — `job->accounts`/`job->allocs` stay `NULL`, a TRACE line is logged, the job is re-checked next round (unchanged behavior, now reached by more jobs per round since the gate widened).
- **No recorded origin anywhere in the replay** (Requirement 4.4): the same "not found" path applies every round for the rest of the replay; the scheduler continues processing without terminating, and that job's resource usage is reported as unavailable (never fabricated) wherever it is inspected (e.g. `info worker`).
- **`decoded_session` decode failure**: unchanged — already handled upstream of this function (`sim_loop.c:874-879`) by skipping verification for that round; this feature does not alter that path.
## Testing Strategy
### Unit Tests (`test_sim_reconcile_unaccounted.c`)
- Replace `test_reconcile_skips_non_run_state_job` with `test_reconcile_captures_non_run_state_job_when_dispatch_recorded` — a `JOB_STAT_PEND` job with a matching recorded DISPATCH entry now gets `job->allocs`/`job->accounts` captured and the entry consumed (Requirement 1.1, 3.1).
- New: `test_reconcile_still_skips_already_captured_job_regardless_of_state` — a job with `job->accounts` already set (any state) is never re-searched, proving the state-widened gate did not weaken the existing no-double-capture guard (Requirement 3.2).
- New: `test_reconcile_no_match_leaves_pend_job_uncaptured_and_retries` — a PEND job with no matching entry this round stays uncaptured and is eligible again next round (Requirement 4.4, state-independent retry).
- New: `test_reconcile_multiple_jobs_captured_independently_across_different_rounds` — several PEND jobs simultaneously outstanding, each with its own recorded DISPATCH decision spread across different rounds (some matching this round, some not yet recorded, some already captured from an earlier round); assert each job's `job->accounts` reflects only its own jobid's allocation data, captured at its own correct round, with zero cross-job interference from the wider per-round scan (Requirement 1.1, 3.1 under concurrent load — the single-job tests above cannot distinguish "correct" from "correct by coincidence, only one candidate available").
- Existing `test_reconcile_reconstructs_allocs_and_accounts_from_matching_dispatch`, `test_reconcile_ignores_non_dispatch_entry_with_matching_jobid`, `test_reconcile_compacts_via_shift_down_preserving_order`, `test_reconcile_is_safe_to_recheck_on_a_later_round`, `test_reconcile_then_release_via_job_state_change_is_asan_clean`, `test_reconcile_null_args_are_a_safe_noop` — re-run unmodified as regression coverage (Requirement 4.1); no expected behavior change for an already-RUN job.
### Integration Tests (round-boundary + charging interaction)
- New: `test_captured_pend_job_excluded_from_resinuse_until_run` — drive `sim_reconcile_unaccounted_jobs()` then `sched_account_jobs()` back to back for a PEND job with a freshly-captured account; assert `w->resInUse`/`core->sharedInUse` stay zero (Requirement 1.2), then transition the job to RUN and re-run `sched_account_jobs()`; assert `resInUse` now reflects it (Requirement 1.3).
- New (resource-contention fixture, per `scheduler-driver-equivalence.md`'s explicit discipline): a worker with capacity for N jobs, more than N candidate jobs recorded, dispatch decisions and RUN confirmations separated across multiple rounds — assert the live allocator's own capacity check (`sched_check_host_satisfy_`) sees correct `resInUse` at each round, not permanently-zero capacity. A no-contention fixture is explicitly insufficient here (`scheduler-driver-equivalence.md`).
- New: `test_sim_verify_no_false_mismatch_across_round_separated_dispatch` — a `--sim-verify` run over a replay where dispatch and RUN confirmation are several rounds apart reports zero mismatches (Requirement 4.2).
- Existing `test_repl_equivalence.c`-style interactive/batch parity coverage re-run unmodified — this feature's call site is shared between both modes, so no new test is needed to prove Requirement 4.3, only confirmation the existing suite still passes.

90
.kiro/specs/scheduler-sim-eager-reconciliation/research.md Normal file
View file

@ -0,0 +1,90 @@
# Research & Design Decisions
## Summary
- **Feature**: `scheduler-sim-eager-reconciliation`
- **Discovery Scope**: Extension (widens an existing, already-shipped mechanism's trigger condition; no new subsystem, no external dependency)
- **Key Findings**:
- The fix is a single-condition removal in `sim_reconcile_unaccounted_jobs()`'s trigger gate (`sim_loop.c:592`), not a new cross-round data structure. The per-round `decoded_session` decode/reconcile/free cycle already runs every round regardless of any job's state; the only reason cross-round matching fails today is that the gate additionally requires `JOB_IS_RUN(job->state)` before a job is even considered for a search, so a job still PEND at the round its own DISPATCH decision appears is skipped outright and never re-checked once it later becomes RUN (by then that round's `decoded_session` is long freed).
- `sched_account_jobs()` (the function that actually turns `job->accounts` into `w->resInUse`/`core->sharedInUse`) already independently gates on `JOB_IS_RUN(job->state)` before charging. This means capturing `job->accounts` early (while a job is still PEND) is inert until the job's real state later transitions to RUN — the existing charging gate is exactly the state-appropriate deferral this feature needs, and it requires no change.
- One existing test, `test_reconcile_skips_non_run_state_job` (`test_sim_reconcile_unaccounted.c:507-531`), asserts the CURRENT (buggy) behavior by name and message ("a non-RUN-state job must never be reconciled here") — this test encodes the bug and must be replaced (inverted), not left alongside new coverage.
## Research Log
### Where does resInUse actually go to zero, and what re-populates it?
- **Context**: bug-1613's buglog root-cause trail (confirmed) plus `.kiro/steering/scheduler-driver-equivalence.md`.
- **Sources Consulted**: `src/scheduler/src/scheduler.c` (`sched_account_jobs`, `sched_commit_decision`), `src/scheduler/src/resource.c` (`sched_update_res_accounts`, `sched_charge_account`), `.wolf/buglog.json` bug-1611/bug-1613 full history.
- **Findings**: `xce_core_begin_session()` zeroes `resInUse`/`sharedInUse` every round; `sched_account_jobs()` is the only re-summing mechanism, iterating `core->jobs` and charging every job that is both `JOB_IS_RUN` and has `job->accounts != NULL`. Normal mode's `sched_commit_decision()` sets `job->accounts` synchronously at DISPATCH-decision time, independent of `job->state` — so by the time a job's state reaches RUN, its accounts are already long-set. Simulation mode has no equivalent synchronous path (its own live picker never emits a DISPATCH — confirmed in the bug-1613 trail — it is PEND-only), so `job->accounts` for a replayed job can only come from `sim_reconcile_unaccounted_jobs()` reconstructing it from the recorded decision stream.
- **Implications**: the fix belongs entirely in how/when `sim_reconcile_unaccounted_jobs()` captures `job->accounts` — `sched_account_jobs()` itself is correct today and must not change.
### Why does today's reconciliation only work same-round?
- **Context**: bug-1611's confirmed root-cause note and the `scheduler-sim-decision-parity` spec's task 4.3 implementation notes (both quoted in `.wolf/buglog.json`).
- **Sources Consulted**: `src/scheduler/src/sim_loop.c:566-709` (`sim_reconcile_unaccounted_jobs`, read in full), `src/scheduler/src/sim_loop.c:840-960` (round-boundary call sequence, read in full).
- **Findings**: `decoded_session` is decoded fresh from the replay stream once per round (`decode_ReplaySession_ptr`, `sim_loop.c:871`) and freed at the end of the same round (`free_ReplaySession_ptr`, `sim_loop.c:1006`) — it never survives past its own round. `sim_reconcile_unaccounted_jobs()` is called unconditionally every round (`sim_loop.c:948`, between `xce_core_begin_session()` and `sched_account_jobs()`), but its trigger gate (`sim_loop.c:592`) is:
```c
if ((job == NULL) || !JOB_IS_RUN(job->state) || (job->accounts != NULL)) {
continue;
}
```
A job whose DISPATCH decision is recorded at round N but whose RUN-state confirmation is only replayed at round N+k (realistic dispatch-to-confirmation latency) is skipped at round N (state is still PEND) and is never re-considered once it reaches RUN at round N+k, because round N's `decoded_session` — the only place that DISPATCH decision ever existed — is already gone.
- **Implications**: no new persistent, cross-round data structure is needed. The function already re-runs every round with a live view of the round's own recorded decisions; it only needs to stop requiring `JOB_IS_RUN` before searching. Removing that one condition means every not-yet-captured job is (re-)checked against every round's own decoded stream until either found (captured, permanently, via the existing `job->accounts != NULL` guard) or the replay ends.
### Does removing the RUN-state gate risk fabricating or double-capturing?
- **Context**: Requirement 3's explicit no-fabrication / no-double-capture acceptance criteria.
- **Sources Consulted**: `sim_loop.c:592-693` (full match/capture logic, read in full).
- **Findings**: The match loop already restricts a capture to the job's own jobid (`entry->body.dispatch->alloc->jobid != job->jobid`, `sim_loop.c:605`) — this is state-independent and unaffected by the gate change. The `job->accounts != NULL` continue-guard (unchanged) already prevents a second capture on any later round, for any job, regardless of state. Widening the state condition therefore only widens *which jobs are eligible to be searched for*, never *what counts as a match* or *whether a second capture can happen*.
- **Implications**: Requirement 3 (no fabrication, no double-capture) is satisfied by logic that already exists and needs no modification — only new test coverage proving it still holds once the gate widens.
### Interaction with `--sim-verify`
- **Context**: Requirement 4.2 (`--sim-verify` must not report a new mismatch caused by this feature).
- **Sources Consulted**: `sim_loop.c:925-1004` (reconciliation call site through the verify positional-walk loop, read in full).
- **Findings**: Reconciliation runs and compacts `decoded_session->decisions` *before* the `--sim-verify` positional walk (`sim_loop.c:970-1004`) reads that same array. Today, a job whose confirmation lands in a later round leaves its DISPATCH entry sitting **unconsumed** in the round-N `decoded_session` (reconciliation skipped it), which `--sim-verify`'s walk would then see as a recorded entry with no matching live decision — a latent false-mismatch risk in `--sim-verify` mode that already exists independent of this fix. Widening the gate causes reconciliation to consume that entry at the correct round instead, which removes this latent risk rather than introducing a new one.
- **Implications**: this fix is verify-neutral-to-positive, not verify-risky. Still needs an explicit test per Requirement 4.2's own wording (a `--sim-verify` run across a multi-round-separated dispatch must not report a new mismatch).
### Does early capture risk an incorrect uncharge when a captured-but-not-yet-RUN job later leaves the job set entirely?
- **Context**: capturing `job->accounts` while a job is still PEND (this feature's whole point) means, for the first time, a job can carry non-NULL `accounts` without ever having been charged. If any cleanup path unconditionally treats "accounts non-NULL" as "this job was charged, so subtract it," early capture would cause `resInUse` to under-count (a phantom decrement for a charge that was never added) for any captured-but-never-actually-run job (e.g. cancelled while still PEND).
- **Sources Consulted**: every production (non-test) reference to `job->accounts`/`sched_uncharge_account` across `src/scheduler/src/*.c`, via grep: `resource.c:591-602` (`sched_uncharge_account`/`sched_commit_job_alloc` definitions), `scheduler.c:964-980` (`sched_job_state_change`), `core_events.c:898-921` (`XCE_CORE_EVENT_JOB_STATE_CHANGE` handler) — the two places a job's `job->accounts` gets freed outside of normal completion.
- **Findings**: `sched_uncharge_account()` has **zero production callers** anywhere in the scheduler (confirmed by grep across every `.c` file in `src/scheduler/src/` — its only callers are test files exercising it in isolation). Both places `job->accounts` gets freed on a non-RUN state transition (`scheduler.c:974-977`, `core_events.c:915-918`) do exactly one thing: `free_job_account(job->accounts); job->accounts = NULL;` — memory release only, no `resInUse` mutation, no call to `sched_uncharge_account`. Production accounting is entirely a **reset-and-resum** model, not incremental charge/uncharge: `xce_core_begin_session()` zeroes `resInUse`/`sharedInUse` every round, and `sched_account_jobs()` re-sums it from scratch by summing every *currently* `JOB_IS_RUN` job with non-NULL `accounts`. A job never counts toward `resInUse` until a round where it is both RUN and has accounts — regardless of how early those accounts were captured.
- **Implications**: a job captured while still PEND that later transitions directly to a terminal state (cancelled, failed) **without ever passing through RUN** is simply never summed by `sched_account_jobs()` at any round (it never satisfies `JOB_IS_RUN`), and its `job->accounts` is freed harmlessly with zero `resInUse` impact when its terminal state is applied. Early capture cannot cause an under-count. This is not a risk this design needs to mitigate — it is a consequence of an accounting model (reset+resum, no incremental uncharge) that was already correct before this feature and remains correct with a wider capture-eligibility gate.
### Does bug-1624's normal-mode fix (job->state = RUN set synchronously at dispatch-commit) already close this gap for current recordings?
- **Context**: `bug-1624` (fixed 2026-08-23, `scheduler-dispatch-commit` tasks 2.1/3.1) made `sched_commit_decision()` set `job->state = JOB_STAT_RUN` synchronously in the same operation as `job->accounts`/`job->allocs`, specifically to close a *live* normal-mode over-commit window. Since this predates the current investigation, it's worth confirming it doesn't also incidentally collapse the round-separation this spec fixes for any recording made after 2026-08-23.
- **Sources Consulted**: `grep -n "job->state\|JOB_STAT_RUN" src/scheduler/src/sim_loop.c` (only hit: the trigger gate itself, `sim_loop.c:592`); `src/scheduler/src/scheduler.c:1441-1447` (`sched_commit_decision`, read in full earlier); `.wolf/buglog.json` bug-1624's full fix note.
- **Findings**: `sim_reconcile_unaccounted_jobs()` never sets `job->state` anywhere — replay-mode state transitions come exclusively from the separately-recorded `XCE_CORE_EVENT_JOB_STATE_CHANGE` event stream, applied via `core_events.c`, independent of whatever the decision stream contains. `sched_commit_decision()`'s `job->state = JOB_STAT_RUN` (bug-1624's fix) is a direct in-memory field write in the *live* scheduler process — not a call into the core's event-push pipeline (`xce_core_push_events()`) — so it is not, by itself, a recordable core event. The recorded `JOB_STATE_CHANGE` event a replay later applies still originates from whatever asynchronous signal produces it for recording purposes (the controller's own worker-confirmation path), independent of bug-1624's live in-memory synchronization.
- **Implications**: bug-1624 fixed a live-mode admission-safety window; it does not change what ends up in a recording's event stream or its round timing. This spec's premise — a recorded DISPATCH decision and its recorded RUN-state-change event can land in different replay rounds — holds for recordings made before *and* after bug-1624, and is not something a future reader should mistake as already resolved by that unrelated fix.
### Existing test locking in the bug
- **Context**: identifying what existing coverage this feature must invalidate, not just extend.
- **Sources Consulted**: `src/scheduler/testsuite/test_sim_reconcile_unaccounted.c` (grepped test names, read `test_reconcile_skips_non_run_state_job` in full at lines 507-531).
- **Findings**: `test_reconcile_skips_non_run_state_job` adds a `JOB_STAT_PEND` job with a matching recorded DISPATCH entry and asserts `job->accounts` stays `NULL`, with the message "a non-RUN-state job must never be reconciled here, regardless of what recorded data exists for it this round." This is the literal bug this spec fixes, encoded as a passing test.
- **Implications**: this test must be replaced (inverted expectation, renamed), not left in place alongside new coverage — the design's File Structure Plan and the eventual task list must call this out explicitly, or the fix will be blocked by its own existing test suite.
## Architecture Pattern Evaluation
| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| Widen existing gate (chosen) | Remove `!JOB_IS_RUN(job->state)` from `sim_reconcile_unaccounted_jobs()`'s trigger condition; rely on the function's existing every-round re-invocation and the existing `job->accounts != NULL` guard for persistence/idempotency | Minimal surface area (one condition), reuses every existing invariant (dedup, ownership transfer, decision reporting) untouched, symmetric with normal mode's own state-independent capture | Every not-yet-captured job is now re-searched against every round's decoded stream until found — O(jobs × rounds) worst case for a job with no recorded origin at all (Req 4.4 case) | Chosen: smallest design satisfying all requirements (design-synthesis Simplification lens) |
| New persistent cross-round pending-capture table | Maintain a separate `jobid -> pending decision` map populated across rounds, independent of `sim->core.jobs` iteration | Could in principle capture before a job even exists in `core->jobs` | Solves a problem that does not exist here — `decoded_session` already only ever contains decisions for jobs `sim->core.jobs` already knows about at that round; adds a new owned lifetime/lookup structure and a new invalidation surface for no behavioral gain | Rejected — violates the Simplification lens; not pursued further |
## Design Decisions
### Decision: Remove the `JOB_IS_RUN` pre-filter instead of adding a new capture-tracking structure
- **Context**: bug-1611/1613's residual gap is that reconciliation only matches within the same round a job happens to already be RUN.
- **Alternatives Considered**:
1. New persistent `jobid -> captured allocation` structure spanning rounds, decoupled entirely from `sim->core.jobs` iteration.
2. Widen the existing trigger gate on the existing per-round iteration (chosen).
- **Selected Approach**: Delete `!JOB_IS_RUN(job->state)` from the trigger condition at `sim_loop.c:592`; leave every other line of the function (match search, ownership transfer, decision reporting, compaction) unchanged.
- **Rationale**: The function already iterates `sim->core.jobs` every round and already searches that round's own decoded stream; the only defect is an overly-narrow precondition on which jobs are eligible. `sched_account_jobs()`'s own unchanged RUN-state gate already provides the correct "don't charge until actually running" deferral this feature's Requirement 1.2 needs — building a second deferral mechanism would duplicate an already-correct one.
- **Trade-offs**: A job that never has a recorded origin anywhere in the replay (Requirement 4.4) is now re-searched, and its "not found" TRACE-level log line re-emitted, every round for the rest of the replay instead of never being searched again after one failed same-round attempt. Accepted: TRACE-level (filtered by default log level in normal operation) and structurally required by Requirement 4.4's "shall continue processing... without terminating" wording, which implies retrying, not giving up.
- **Follow-up**: Verify during implementation that no other caller or test assumes today's "same-round-only" contract as load-bearing behavior beyond the one test identified above.
## Risks & Mitigations
- **Existing test asserts the bug as correct behavior** (`test_reconcile_skips_non_run_state_job`) — Mitigation: replace it with an inverted-expectation test in the same task that changes the gate, not as a follow-up; do not let old and new expectations coexist even briefly.
- **Resource-contention blind spot** (`scheduler-driver-equivalence.md`'s own documented testing discipline: a no-contention fixture cannot distinguish "accounting correct" from "accounting wrong but unread") — Mitigation: Testing Strategy must include at least one fixture where a worker's capacity is insufficient for all candidate jobs, so a wrongly-zero `resInUse` would visibly change a scheduling decision, not just a REPL display value.
- **`--sim-verify` false-mismatch risk already latent today** — Mitigation: add an explicit `--sim-verify` regression test spanning a multi-round dispatch-to-confirmation gap; confirm it reports zero mismatches after the fix (and, ideally, confirm it currently free of new mismatches — the fix is expected to strictly improve this, not just avoid regressing it).
- **Log noise from repeated "not found" TRACE lines** across many rounds for a job with no recorded origin — Mitigation: accepted as documented trade-off (see Design Decision above); TRACE level already filtered in default operation, not escalated further by this spec.
- **Wider per-round search cost**: every not-yet-captured job (previously only RUN-state ones) is now linearly searched against that round's decoded decision list every round until captured or the replay ends — a round with a large PEND backlog does more search work than before. Mitigation: the per-job, per-round search cost itself is unchanged (same linear scan the existing mechanism already performs); only the set of jobs subjected to it widens. Replay is a debugging/tuning tool, not a real-time or production hot path, and `--sim-speed` already exists to control replay pacing — accepted without further mitigation; revisit only if a real replay is observed to become impractically slow.
## References
- `.kiro/steering/scheduler-driver-equivalence.md` — the enforced state-equivalence invariant and its testing discipline this feature closes the remaining gap in.
- `.kiro/specs/scheduler-sim-decision-parity/` — the parent spec whose `sim_reconcile_unaccounted_jobs()` mechanism this feature extends (implementation-complete, not reopened).
- `.wolf/buglog.json` bug-1611, bug-1613 — full confirmed root-cause and partial-fix history.

14
.kiro/specs/scheduler-sim-eager-reconciliation/spec.json
View file

@ -3,20 +3,20 @@
"created_at": "2026-08-22T14:50:13Z",
"updated_at": "2026-08-22T14:58:00Z",
"language": "en",
"phase": "requirements-generated",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": true,
"approved": false
"approved": true
},
"design": {
"generated": false,
"approved": false
"generated": true,
"approved": true
},
"tasks": {
"generated": false,
"approved": false
"generated": true,
"approved": true
}
},
"ready_for_implementation": false
"ready_for_implementation": true
}

52
.kiro/specs/scheduler-sim-eager-reconciliation/tasks.md Normal file
View file

@ -0,0 +1,52 @@
# Implementation Plan
- [ ] 1. Core: widen reconciliation's capture eligibility to any not-yet-captured job
- [x] 1.1 Prove and implement state-independent capture of a job's recorded dispatch allocation
- Replace the existing test that locks in the bug (asserting a not-yet-running job must never be reconciled) with one asserting the opposite: a job whose recorded dispatch decision is found this round gets its allocation captured regardless of whether it has reached a running state yet
- Run the new/updated test against the current code first to confirm it fails for the expected reason (the old state-gated behavior), then remove the state precondition from the capture-eligibility check so only "not yet captured" remains
- Update the function's own explanatory comments so they describe the new state-independent trigger instead of the old state-gated one
- Observable: the updated unit test suite shows the not-yet-running job's allocation is captured and its recorded entry consumed, where it previously stayed uncaptured
- _Requirements: 1.1, 3.1_
- _Boundary: sim_reconcile_unaccounted_jobs_
- [ ] 1.2 Extend unit coverage for capture-safety guarantees under the widened eligibility
- Add a test proving a job whose allocation was already captured is never re-searched or re-captured on a later round, regardless of its state
- Add a test proving a job with no matching recorded decision this round stays uncaptured and remains eligible to be found on a later round
- Add a test with several simultaneously outstanding jobs whose recorded dispatch decisions land in different rounds, proving each job's captured allocation reflects only its own recorded decision with no cross-job interference
- Observable: the unit test suite demonstrates no-double-capture, retry-on-no-match, and per-job isolation all hold under the widened eligibility check
- _Requirements: 3.1, 3.2, 4.1, 4.4_
- _Boundary: sim_reconcile_unaccounted_jobs_
- [ ] 2. Integration: prove captured-but-not-running allocations stay excluded from resource-in-use accounting until the job actually runs
- [ ] 2.1 Verify resource-in-use accounting defers to the job's real running state, not its capture time
- Drive a captured, not-yet-running job's allocation through the resource-in-use recomputation and confirm it contributes nothing to worker or shared resource usage
- Transition that same job to a running state and drive resource-in-use recomputation again, confirming its allocation now contributes from that point forward
- Observable: a test demonstrates the same captured job's resource usage is zero before its running-state transition and correctly non-zero after, using the exact allocation captured earlier
- _Requirements: 1.2, 1.3, 1.4_
- _Boundary: sim_reconcile_unaccounted_jobs, sched_account_jobs_
- _Depends: 1.1_
- [ ] 3. Validation: end-to-end proof under contention, decision-verification mode, and full regression
- [ ] 3.1 (P) Verify correct capacity accounting under resource contention with realistic dispatch-to-confirmation separation
- Build a replay fixture where a worker's capacity is insufficient for every candidate job, with dispatch decisions and their running-state confirmations recorded several rounds apart
- Confirm the live capacity check sees the correct, non-zero resource-in-use total at each round rather than treating capacity as permanently free
- Observable: a resource-contention test demonstrates the capacity check's view of resource usage matches what actually happened in the original recording at each round, not a permanently-zero reading
- _Requirements: 1.1, 1.2, 1.3, 1.4_
- _Boundary: sim_reconcile_unaccounted_jobs, sched_account_jobs_
- _Depends: 1.1_
- [ ] 3.2 (P) Verify decision-verification mode reports no new mismatch across a round-separated dispatch
- Run decision-verification mode over a replay where a job's dispatch decision and its running-state confirmation are recorded several rounds apart
- Confirm the verification pass reports zero mismatches for that job's decision
- Observable: a decision-verification test run over a multi-round-separated dispatch scenario reports zero mismatches
- _Requirements: 2.1, 2.2, 2.3, 4.2_
- _Boundary: sim_reconcile_unaccounted_jobs_
- _Depends: 1.1_
- [ ] 3.3 Confirm full regression: existing reconciliation, interactive/batch parity, and the full scheduler suite are unaffected
- Re-run the existing reconciliation unit tests that were not modified by this feature and confirm their already-running-job scenarios still pass unchanged
- Re-run the existing interactive-versus-batch simulation parity coverage and confirm it still passes unchanged
- Run the full scheduler test suite and confirm no new failures beyond any already-tracked, unrelated pre-existing ones
- Observable: a full scheduler test suite run shows the same pass/fail baseline as before this feature, with only the new tests added by this feature as net-new coverage
- _Requirements: 3.3, 4.1, 4.3, 4.5_
- _Depends: 3.1, 3.2_