feat(scheduler-sim-decision-parity): report reconciled decisions through the existing channel (task 2.4)

Feeds a reconciled job's decision (task 2.3) through the exact same
recording/print/REPL-check calls the existing decision-drain loop uses for
a freshly-drained decision, via make_dispatch_decision() -- the same
DISPATCH-construction idiom the live allocator path already uses.
Dedup (Requirement 3.3) needs no new mechanism: the job->accounts gate
from tasks 2.2/2.3 already prevents double-reporting, now proven at the
reporting layer too (zero recorded decisions / no console output for an
already-accounted job).

Required extending sim_reconcile_unaccounted_jobs()'s signature with a
third parameter, a nullable recording-context handle -- design.md's
Service Interface updated to match, with the confirmed reasoning: the
handle only exists as a stack-local inside the round loop, and task 3's
still-unwired call site sits in that same scope, so this is necessary
plumbing, not scope creep.

46/47 scheduler tests pass; the 1 failure is pre-existing bug-1560
(unrelated).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-22 09:07:24 +00:00
parent eccba84112
commit 0c237f601a
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
4 changed files with 278 additions and 61 deletions

View file

@ -196,11 +196,23 @@ int sched_account_jobs(xce_sched_core_t* core);
* this is deliberate, not an oversight: the caller's own subsequent
* --sim-verify positional walk over the same decoded_session depends on
* consumed entries already being gone by the time it runs.
*
* record_ctx is nullable and mirrors the existing decision-drain loop's
* own `if (record_ctx != NULL)` gate (this reporting path must recording-
* capture identically to a freshly-drained decision, and recording is
* optional per run) -- CONFIRMED during implementation this third
* parameter is necessary, not avoidable: record_ctx exists only as a
* stack-local inside sched_sim_driver_run()'s per-ReplaySession block
* (no field on xce_sched_sim_driver_t can supply it), and the round-
* boundary call site this function is wired into (task 3) sits in that
* exact same lexical scope, so passing it through is the natural,
* unambiguous plumbing rather than a workaround.
*/
void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
replay_session_t* decoded_session);
replay_session_t* decoded_session,
sched_record_session_ctx_t* record_ctx);
```
- Preconditions: `sim`/`decoded_session` non-NULL (NULL is a safe no-op, matching this codebase's established "never load-bearing" REPL-hook convention).
- Preconditions: `sim`/`decoded_session` non-NULL (NULL is a safe no-op, matching this codebase's established "never load-bearing" REPL-hook convention). `record_ctx` may be NULL (recording not active for this run).
- Postconditions: every RUN-state job either has `job->accounts` set (reconstructed) or is left NULL (genuinely unavailable this round). `decoded_session->decisions`/`n_decisions` reflect the removal of every consumed entry (freed, not merely uncounted -- see memory-ownership note below), leaving the remaining entries in their original relative order.
- Invariants: never double-applies (idempotent per job, per the `job->accounts` gate); never fabricates allocation data not present in the recording; never leaves a freed pointer reachable through `job->allocs` or through the remaining `decoded_session->decisions` entries.

View file

@ -45,7 +45,7 @@
- _Depends: 2.2_
- _Boundary: Recorded-Decision Reconciliation_
- [ ] 2.4 Make a reconciled job's decision visible the same way a fresh one is
- [x] 2.4 Make a reconciled job's decision visible the same way a fresh one is
- Feed a reconstructed decision (task 2.3) through the exact same decision-reporting path a freshly-computed decision already uses, preserving its original decision type and resource-allocation detail
- If simulation mode's own live decision-making independently produces a fresh decision for a job in the same round it would otherwise be reconciled, report only the fresh one — never both
- Observable completion: replaying a fixture where a job was already dispatched before the replay's live decision-making reaches it now shows that job's dispatch decision in step/continue output, exactly once, with the same resource-allocation detail the original recording captured
@ -94,3 +94,5 @@
- (task 2.3) Do NOT check "not found" via `i == decoded_session->n_decisions` after the lookup loop once the loop's array can be mutated mid-iteration (e.g. once `n_decisions` gets decremented by a compaction) -- that check goes stale for a last-index match. Use an explicit `found` flag instead. This bug would only have surfaced later (task 2.3 introduced the mutation that makes the old check wrong) but was caught and fixed here.
- (task 2.3) `src/lib/misc.c`'s `free_host_account()` never frees the `strdup`'d `host` field `make_host_account()` (`src/lib/xcealloc.c:119`) allocates -- a real, pre-existing leak, equally present on the live `scheduler.c` dispatch/release path, previously masked because this test suite is registered with `ASAN_OPTIONS=detect_leaks=0`. Logged as bug-1621, intentionally NOT fixed here (out of this feature's boundary -- misc.c isn't owned by scheduler-sim-decision-parity). If a later task's ASAN run surfaces this same leak, it's not a new regression -- don't waste time re-diagnosing it, it's bug-1621.
- (task 2.3) `make_job_account()`'s return isn't NULL-checked before use, matching the live-path precedent (`scheduler.c`'s `sched_commit_decision()` has the same gap) -- informational only, not a defect to fix in this feature unless the live path itself gets hardened first.
- (task 2.4) The reconciliation function ended up needing a third parameter (a nullable recording-context handle) beyond the two design.md originally specified, because that handle only exists as a stack-local inside the round-loop and nothing on the driver struct can supply it -- design.md's Service Interface has been updated to match. Task 3 (wiring): the call site sits in the exact same lexical scope where that recording-context variable is already declared, so pass it straight through -- no new plumbing needed.
- (task 2.4) `xce_decision_t` objects built via `make_dispatch_decision()`/`make_pend_decision()` are never freed anywhere in this codebase on any live path (verified by grep across `scheduler/src/*.c`) -- this is an established, accepted pattern here, not something to "fix" if you see it under `detect_leaks=1`. Don't add a free for it; that would deviate from the codebase's own convention without a broader cleanup pass.