feat: implement Phase 4d const-fold transform for select AST

Adds sel_const_fold_pass() — a single-pass bottom-up const-fold that
runs after the Phase 4c flatten loop in parse_select():

  4d-1  EQ/NE with either operand SEL_ANY → SEL_TRUE / SEL_FALSE
        (checks both child slots — commutativity sort makes SEL_ANY
        position non-deterministic after makeSelectNode interning)
  4d-2  Identity laws: TRUE&&X→X, X&&TRUE→X, FALSE||X→X, X||FALSE→X
  4d-3  Annihilation laws: FALSE&&X→FALSE, TRUE||X→TRUE (symmetric)
  4d-4  Complement laws: !TRUE→FALSE, !FALSE→TRUE

Memory semantics:
  level==0 (API server): frees orphaned nodes via free_select_node /
    sel_free_tree; OOM from makeSelectNode returns original node safely
  level>0  (CSE/scheduler): never manually frees; annihilation folds
    return the already-interned left/right singleton directly (no
    redundant makeSelectNode call); OOM guard unconditional so CSE
    mode also degrades gracefully instead of returning NULL

9 new TDD tests added to test_select.c (RED→GREEN verified).
All 166 tests pass (157 prior + 9 Phase 4d).
This commit is contained in:
Rongsong Shen 2026-04-29 08:24:04 +00:00
parent c2ad96a5e0
commit b38babb7d6
9 changed files with 3248 additions and 5 deletions

View file

@ -0,0 +1,71 @@
# P1-2: Host Resource Requirement Select — Investigation
## Status
- [>] Under investigation (moved from apiserver-backend-tasks.md)
---
## Background
The `select` field in an LSF-style resource requirement string (e.g. `select[mem>4096 && type==X86_64]`)
filters candidate hosts before slot allocation. Currently `EvalSelectForHost` is a no-CGO stub that
always returns `errNoCGO`.
The full feature requires two layers:
1. **C parser + evaluator** — parse the boolean select expression into an AST and evaluate it against
a host's resource values.
2. **Go bridge** — call the C evaluator via CGO and wire it into `FilterHostByResReqSelect` in the
controllers layer.
---
## Sub-tasks
### #11 P1-2a: C select expression parser (PEG + eval)
**Files to touch:**
- `include/resreq.h` — Replace empty `xce_resreq_select_` stub with expression-tree types, enums,
and `extern` declarations for `parse_select`, `eval_select`, `free_select`.
- `lib/xce_select.peg` — New PEG grammar for select boolean expressions (right-recursive, handles
`&&`, `||`, `!`, comparison operators, numeric literals, string literals, resource names).
- `lib/resreq.c` — Implement `parse_select()`, `eval_select()`, `free_select()`; update
`kill_primary_resreq()` to call `free_select()`.
- `lib/meson.build` — Add `xce_select.peg` to explicit source list so the PEG tool generates C.
- `lib/testsuite/` — TDD tests for parse/eval before implementing.
**Open questions:**
- Which PEG library is already available in the Nix/Meson build? (`peg`, `lemon`, `re2c`?)
- Exact grammar for comparison operators — LSF allows `mem > 4096`, `type == X86_64`, `swp >= 0`.
Need to confirm all supported operators and value types.
- Does `eval_select` receive a flat key-value map of resource values, or a pointer to the full
`xce_host_t` struct?
- Error reporting strategy — return a bool+error-string, or a tri-state (true/false/parse-error)?
### #12 P1-2b: Go EvalSelectForHost + FilterHostByResReqSelect (blocked by #11)
**Files to touch:**
- `backend/clib.go` — Implement `EvalSelectForHost(hostname, selectExpr string) error` using
`cgo.Handle` for callback context; add `#include "resreq.h"`, extern C declarations,
`xce_select_destroy` helper.
- `controllers/hosts.go` — Implement `FilterHostByResReqSelect(hosts []models.HostInfo, expr string)`,
update `preCheckHostRequest()` limit from 4094 → 1024 bytes.
**Open questions:**
- Does `EvalSelectForHost` need to look up live resource values from the in-memory store, or are
values passed in directly by the C layer?
- `cgo.Handle` usage pattern — confirm whether a per-call handle or a session-level handle is needed.
- Input validation: max expression length (1024 bytes), allowed characters.
---
## Investigation Checklist
- [ ] Survey existing PEG tooling in the Meson/Nix build environment.
- [ ] Read `include/resreq.h` current state to understand `xce_resreq_select_` stub.
- [ ] Trace how `kill_primary_resreq()` works today and where `free_select()` must be called.
- [ ] Confirm resource value structure passed to `eval_select`.
- [ ] Confirm LSF select grammar spec (operators, value types, nesting depth).
- [ ] Prototype PEG grammar for a subset (numeric comparisons only) and get it compiling.
- [ ] Decide error propagation strategy across the CGO boundary.
- [ ] Write TDD tests for C parser before any implementation.

View file

@ -0,0 +1,220 @@
# Resreq AST + CSE Implementation Plan
> Generated from: `resreq-ast-cse-design.md`
> Date: 2026-04-28
> Branch: merge_api_server
---
## Summary Priority Order
```
P0 (blocking): makeString() TOCTOU fix, section.peg boundary bug ✓ DONE
Phase 1: TDD test skeletons (parallel with Phase 3) ✓ DONE
Phase 2: Header definitions + ABI audit ✓ DONE
Phase 3: PEG grammar (parallel with Phase 1) ✓ DONE 143 PASS / 3 FAIL (stubs)
Phase 4a: Core interning + sel_optimize_level ✓ DONE 145 PASS / 1 FAIL (Phase 4b stub)
Phase 4b: Type validation (gated on P0-1) ← CURRENT
Phase 4c/4d: Canonicalization + const-fold
Phase 4e: Memoized eval
Phase 4f: GC + recompaction
Phase 5: Build wiring
Phase 6: Scheduler integration (after all unit tests pass)
```
---
## P0 — Critical Prerequisites (blocking gate)
Must complete before any Phase 4 work begins.
| # | Task | Section | Rationale |
|---|------|---------|-----------|
| P0-1 | ~~**Fix `makeString()` TOCTOU in `src/lib/xstring.c`**~~ **DONE** — extended lock region to cover `makeInstance` + `key_add`, eliminating the TOCTOU window. `key_add` can no longer return `-EEXIST` on a miss path. | §5.4 | Silent NULL return corrupts the interned AST in all §7 numeric-to-string coercions; blocks Phase 4b entirely |
| P0-2 | ~~**Fix `section.peg` clause-boundary grammar**~~ **DONE** — Root cause: the original grammar used multiple ordered-choice alternatives in `select_statement` / `usage_statement` / `span_statement`, but PEG's `e*` always succeeds (matches empty), so alternatives 2+ were dead code (confirmed by packcc generating "defined but not used" for all `pcc_action_select_statement_N` functions). Fix: collapsed each statement rule into a single alternative with a compound negative lookahead covering all subsequent clause keywords. `select_statement` now stops at any of `rusage[`, `span[`, `same[`, `affinity[`, or `]`; `usage_statement` stops at `span[`, `same[`, `affinity[`, or `]`; `span_statement` stops at `same[`, `affinity[`, or `]`. Both implicit (`type==any span[ptile=1]`) and explicit (`select[type==any] span[ptile=1]`) select forms verified by TDD: `test_parse_section3` (implicit), `test_parse_section_explicit_select_with_span`, `test_parse_section_explicit_select_rusage_span`, `test_parse_section_span_not_in_select` — all PASS, no packcc dead-code warnings. | §3.1 | Silent span/rusage content bleed into selectString in all implicit-select + multi-clause inputs |
---
## Phase 1 — TDD Tests (write failing skeletons first) ✓ DONE
**Priority: HIGH — must come before implementation (TDD contract)**
Develop concurrently with Phase 3 (grammar is the test oracle).
| # | Task | Notes |
|---|------|-------|
| 1-1 | ~~Write `lib/testsuite/test_select.c` skeletons: parse valid expressions, unknown resource names (error), type mismatches (error)~~ **DONE** | |
| 1-2 | ~~Write tests: `SELECT_MAX_INPUT_LEN` (8192) overflow, `SELECT_MAX_DEPTH` (64) nesting overflow~~ **DONE** | |
| 1-3 | ~~Write CSE tests: `A&&B` and `B&&A` produce same pointer; two `makeSelectNode` calls with same args return same pointer~~ **DONE** — FAIL until Phase 4a | |
| 1-4 | ~~Write eval tests: numeric `>/<`/`==`, string `==`, bool standalone, `!`/`&&`/`||`, short-circuit~~ **DONE** | |
| 1-5 | ~~Write GC tests: unlink job root → `gc_sweep_select_nodes` frees unreachable node; subsequent `makeSelectNode` re-interns fresh node~~ **DONE** — FAIL until Phase 4f | |
| 1-6 | ~~Write GC predicate contract tests (`sched_session_active`, `sched_eval_in_progress`, `staging_merge_complete`) using `fake_scheduler_t` scaffold in `test_select.c`~~ **DONE** — FAIL until Phase 4f | Phase 6 wires real scheduler — signatures stable |
| 1-7 | ~~Create shared resmgr fixture: `src/lib/testsuite/test_resmgr_fixture.h` + `.c` with `make_test_resmgr()`/`free_test_resmgr()`~~ **DONE** | |
---
## Phase 2 — Header Definitions ✓ DONE
**Priority: HIGH — unblocks all implementation work**
| # | Task | Notes |
|---|------|-------|
| 2-1 | ~~Add `res_type_t`, `sel_node_type_t`, `xce_select_node_t` typedef + struct to `include/resreq.h`~~ **DONE** | |
| 2-2 | ~~Fill `struct xce_resreq_select_` with `xce_select_node_t* root`~~ **DONE** | |
| 2-3 | ~~Declare all public APIs: `makeSelectNode`, `eval_select`, `eval_select_simple`, `select_eval_ctx_*`, `sel_node_table_*`, `gc_sweep_select_nodes`, `recompact_node_ids`~~ **DONE** | `gc_mark_tree` stays `static` in `.c` |
| 2-4 | ~~Add `var resmgr` field to `resreq_context_t` (outer PEG auxil)~~ **DONE** | |
| 2-5 | ~~Add `valtype_to_res_type` helper; omit `res_type_to_valtype` (no current consumer — YAGNI)~~ **DONE** | |
| 2-6 | ~~Add `xce_str_match` as `static inline` wrapping `strcmp`~~ **DONE** | |
| 2-7 | ~~**CGO audit**: `grep -rn 'parse_resreq\|C\.parse_resreq' src/frontend/`~~ **DONE** — no CGO callers found | |
| 2-8 | ~~Update `parse_resreq_primary` and `parse_resreq` signatures to accept `var resmgr`~~ **DONE** — call sites updated: `schedobj.c:171`, `test_resreq.c:244/301/330` | |
---
## Phase 3 — PEG Grammar (`lib/xce_select.peg`) ✓ DONE
**Priority: HIGH — develop concurrently with Phase 1**
| # | Task | Notes |
|---|------|-------|
| 3-1 | ~~Implement `select_expr` grammar (§3.1): `\|\|`, `&&`, `!`, comparisons, resname, literals, `any`~~ **DONE** | |
| 3-2 | ~~Implement `select_context_t` auxil with `resmgr`, `depth`, `errmsg[256]`, `has_error`~~ **DONE** | |
| 3-3 | ~~Implement `resname` action: `key_find`, `res_idx` validity guard, `valtype_to_res_type` mapping~~ **DONE** | |
| 3-4 | ~~**Clause-keyword conflict check** in `resname` action: check against `{select, rusage, span, same, affinity}`~~ **DONE** | |
| 3-5 | ~~Implement `SELECT_MAX_INPUT_LEN` (8192) and `SELECT_MAX_DEPTH` (64) guards~~ **DONE** | |
| 3-6 | ~~LSF-compatible disambiguation: bare word vs known resource name → `SEL_RESNAME` vs `SEL_STR`; `any` excluded by grammar; `true`/`false` produce migration diagnostic~~ **DONE** | |
| 3-R1 | ~~**[Review fix — HIGH]** Switch `SEL_STR` from `strdup`/`free` to `makeString`/`dereference` in `xce_select.peg` and `xce_select.c`~~ **DONE** — xstring-interned strings now refcounted; `kill_select_node` calls `dereference()` | |
| 3-R2 | ~~**[Review fix — MEDIUM]** Add `_Static_assert(sizeof(uintptr_t) >= sizeof(double))` before `makeSelectNode` in `xce_select.c`~~ **DONE** — guards the `SEL_NUM` bit-cast at compile time | |
| 3-R3 | ~~**[Review fix — MEDIUM]** Add `!("any" ![a-zA-Z_0-9])` negative lookahead to `resname_node` and `resname_leaf` in `xce_select.peg`~~ **DONE** — prevents `any` keyword from being tried as a resource name | |
---
## Phase 4a — Core Node Allocation and Interning ✓ DONE
**Priority: HIGH — foundational; unblocks 4b–4f**
| # | Task | Notes |
|---|------|-------|
| 4a-1 | ~~Implement `sel_optimize_level` (`static _Atomic int`, sentinel -1) with `sel_optimize_set()` write-once CAS and `sel_get_optimize_level()` accessor~~ **DONE** | |
| 4a-2 | ~~Implement `sel_make_key()` — zero padding bytes via struct literal init; SEL_RESNAME zeroes `op2` for key purposes (res_type stored in node, not in key)~~ **DONE** | |
| 4a-3 | ~~Implement `sel_key_hash` over the 3 key fields~~ **DONE** | |
| 4a-4 | ~~Implement `sel_node_table_init` / `sel_node_table_fini` — `makeInstance(&kvtab_type, &params)` with **`datakiller=NULL`**, `zarray_init` for linear nodes array~~ **DONE** | |
| 4a-5 | ~~Implement `makeSelectNode` — CSE-disabled fast path; CSE-enabled path with commutativity sort, key lookup, TOCTOU double-check under lock, node_id assignment via `atomic_fetch_add`, zarray rollback via `atomic_fetch_sub`~~ **DONE** | |
| 4a-6 | ~~Implement `sel_node_to_key()` for GC key reconstruction (must apply same commutativity sort as `makeSelectNode`)~~ **DONE** | |
| 4a-7 | ~~Add `sel_optimize_test_reset()` test-only helper (`#ifdef UNIT_TEST`): `atomic_store_explicit` reset to -1 plus `sel_node_table_fini`~~ **DONE** | |
| 4a-R1 | ~~**[Review fix — HIGH]** Move `node_id` assignment inside lock after `key_add` succeeds so TOCTOU losers need no counter rollback~~ **DONE** | Counter divergence under concurrent load |
| 4a-R2 | ~~**[Review fix — HIGH]** Add `dereference((char*)op1)` on first-lock cache hit for SEL_STR — caller transfers xstring ownership~~ **DONE** | Ref leak on every cache hit for string nodes |
| 4a-R3 | ~~**[Review fix — HIGH]** Add `SEL_MAX_INTERNED_NODES` cap check in both lock sections (first miss + TOCTOU re-check)~~ **DONE** | Unbounded table growth without cap |
| 4a-R4 | ~~**[Review fix — HIGH]** Fix `sel_node_table_fini` to iterate and free all interned nodes before `destroy(table)` — NULL datakiller does not free them~~ **DONE** | All nodes leaked at teardown; LSAN clean now |
| 4a-R5 | ~~**[Review fix — MEDIUM]** Set `node->hashcode = sel_key_hash_fn(&key)` in CSE path — was left 0, breaking Phase 4e memoization~~ **DONE** | |
| 4a-R6 | ~~**[Review fix — MEDIUM]** Add `dereference((char*)op1)` on `calloc` failure in level==0 path~~ **DONE** | xstring ref leak on OOM |
| 4a-R7 | ~~**[Review fix — MEDIUM]** Add `-EINVAL` guard on `key_find` — corrupt table must not silently proceed~~ **DONE** | |
| 4a-R8 | ~~**[Review fix — MEDIUM]** Add `-EEXIST` recovery in `key_add` failure path — re-find and return the racing winner~~ **DONE** | Rare concurrent insert race |
| 4a-R9 | ~~**[Review fix — LOW]** Change `_Static_assert(sizeof(uintptr_t) >= ...)` to `==` per spec requirement~~ **DONE** | |
---
## Phase 4b — Parse-Time Type Validation
**Priority: HIGH — gate: P0-1 `makeString()` fix must be merged first**
| # | Task | Notes |
|---|------|-------|
| 4b-1 | Wire `parse_select()` into `parse_resreq_primary` **before** `kill_resreq_data` | Use-after-free if called after `kill_resreq_data` frees `data.selectString` |
| 4b-2 | Implement EQ/NE type validation: reject BOOL×STR; coerce SEL_NUM to SEL_STR when comparing with string resource (`snprintf "%.17g"`), NULL-check `makeString` result, symmetric coercion | Free orphaned SEL_NUM node in unoptimized mode before overwriting pointer |
| 4b-3 | Implement `>/<`/`>=`/`<=` type validation: reject non-numeric operands | `log_fatal` (not `assert`) for belt-and-suspenders at eval time too |
| 4b-4 | Implement `res_idx` validity guard and unknown-resource error; add migration diagnostic for `true`/`false` identifiers (parse as SEL_RESNAME with no match) | |
| 4b-5 | Update `kill_primary_resreq` for conditional teardown: CSE-enabled → null root only; CSE-disabled → `free_select_tree()` then `FREEUP` | NULL store before `FREEUP` to prevent GC mark dangling-pointer race |
| 4b-6 | **LC_NUMERIC enforcement**: wrap PEG parser call in `parse_select` with `newlocale(LC_NUMERIC_MASK, "C")` / `uselocale` / `freelocale`; `log_fatal` on failure in scheduler, `log_error`+return in API server | `_XOPEN_SOURCE 700` required before system headers |
| 4b-7 | **strtod locale coverage check**: `grep -n 'strtod\|atof\|strtof' build/lib/xce_select.c` — if packcc inserts `strtod` in scanner, use `%0` capture + explicit `strtod` inside PEG action | Document finding (matches or no-matches) as a confirmed safe/unsafe result |
---
## Phase 4c — Commutativity + Associativity Flattening
| # | Task | Notes |
|---|------|-------|
| 4c-1 | Verify commutativity sort in `makeSelectNode` step 1 covers AND/OR/EQ/NE; non-commutative GT/LT/GE/LE/NOT unchanged | Already integrated in §5.4 design |
| 4c-2 | Implement post-parse **flatten pass** for `&&`/`||` chains: `do { changed = flatten_pass(root); } while (changed)` | Repeat-until-stable needed for depth-3+ explicit parenthesization |
| 4c-3 | In unoptimized mode: `flatten_pass` must `free()` each orphaned node — not its children (re-parented children must not be freed) | Children freed would cause double-free when `free_select_tree` walks from root |
---
## Phase 4d — Const-Fold Transform
| # | Task | Notes |
|---|------|-------|
| 4d-1 | `resname == any` → `SEL_TRUE`; `resname != any` → `SEL_FALSE` — check **both** child slots for SEL_ANY pointer (commutativity sort is ASLR-nondeterministic across runs) | |
| 4d-2 | Identity laws: `SEL_TRUE && X` → `X`; `SEL_FALSE \|\| X` → `X` | |
| 4d-3 | Annihilation laws: `SEL_FALSE && X` → `SEL_FALSE`; `SEL_TRUE \|\| X` → `SEL_TRUE` | |
| 4d-4 | Complement laws: `!SEL_TRUE` → `SEL_FALSE`; `!SEL_FALSE` → `SEL_TRUE` | Single-pass — const-fold shrinks trees, does not create new flattening opportunities |
---
## Phase 4e — Evaluation with Memoization
| # | Task | Notes |
|---|------|-------|
| 4e-1 | Implement `eval_get_bool`/`eval_set_bool` 2-bit bitmap accessors with `node_id >= bool_bits_cap` guards using `log_fatal` | `_Static_assert(EVAL_NONE == 0)` for memset-reset correctness |
| 4e-2 | Implement `eval_select_inner` — recursive eval with **explicit** error propagation (`if (lv < 0) return -1`), NOT C `&&`/`||` (C's `-1` is truthy, would swallow errors) | SEL_ANY reaching eval → `log_fatal("SEL_ANY reached eval — const-fold not applied")` |
| 4e-3 | Leaf cache writes in `eval_select_inner`: SEL_RESNAME writes to `ctx->num_vals[node_id]` or `ctx->str_vals[node_id]`; comparison ops call `eval_select(child, ctx)` then read cached arrays | **Critical**: `get_leaf_value()` must NOT be called directly — bypasses cache writes, silently reads 0.0/NULL |
| 4e-4 | Implement memoized `eval_select` wrapper — bitmap cache check before calling `eval_select_inner`; cache EVAL_ERROR too (node-structural errors repeat per host) | |
| 4e-5 | Implement `eval_select_simple` for unoptimized path — plain recursive tree-walk, no `select_eval_ctx_t`, thread-safe (no shared state) | |
| 4e-6 | Add `eval_select` guard: `log_fatal` if `sel_get_optimize_level() <= 0` | |
| 4e-7 | Implement `select_eval_ctx_init` / `select_eval_ctx_reset` / `select_eval_ctx_fini` | `node_count` snapshot at init; `reset` uses snapshot, never re-reads `sel_node_next_id` |
| 4e-8 | Pre-session capacity check: `memset` extended bool_bits bytes after realloc (realloc does not zero extension); partial-OOM contract — all three reallocs (bool_bits, num_vals, str_vals) must succeed atomically or `log_fatal` | Mismatched capacities allow out-of-bounds write to num_vals with no inline bounds check |
---
## Phase 4f — Mark-and-Sweep GC + Recompaction
| # | Task | Notes |
|---|------|-------|
| 4f-1 | Implement `gc_mark_tree` — recursive DFS, early return on `node == NULL \|\| node->gc_mark`; handles leaf / SEL_NOT / binary node types | Recursion bounded by `SELECT_MAX_DEPTH` (64); any increase requires stack analysis |
| 4f-2 | Implement `gc_sweep_select_nodes` — mark pass (clear all gc_mark + mark from live jobs dispatching on XCE_RESREQ_PRIMARY/COMPOUND/ALTER), sweep pass (collect unmarked, `key_del`, `dereference(strval)`, `free(node)`, `zarray_del`) | Use `xlist_get` with counted loop for xlist iteration; GC does NOT acquire `sel_node_table.lock` (idle-phase invariant) |
| 4f-3 | Insert **`atomic_thread_fence(memory_order_seq_cst)`** after all `primary->select->root = NULL` unlinks and before `gc_sweep_select_nodes` call | Without fence: CPU/compiler may reorder NULL store after GC mark pass |
| 4f-4 | Runtime GC idle invariant guards at GC call site (`sched_session_active`, `sched_eval_in_progress`, `staging_merge_complete`) — `log_fatal` not `assert` | `assert` compiled out by NDEBUG; use-after-free would result |
| 4f-5 | Implement `recompact_node_ids` — iterate `nodes` zarray over `caps` slots (not `count`, gaps exist after `zarray_del`), assign sequential `node_id`, reset `sel_node_next_id` via `atomic_store_explicit(..., memory_order_release)` | `node_id` and zarray slot index are **decoupled** — do not conflate |
---
## Phase 5 — Build Wiring
| # | Task | Notes |
|---|------|-------|
| 5-1 | Add `custom_target` for `xce_select.peg` in `lib/meson.build` matching `section_parser`/`rusage_parser` pattern | Verify `-o 'lib/xce_select'` path format: `grep -A8 'custom_target' src/lib/meson.build \| grep '\-o'` |
| 5-2 | Add `xce_select_parser[0]` (generated `.c`) to `libxce_src` | |
| 5-3 | Add `test_resmgr_fixture.c` to testsuite `executable()` sources in `lib/meson.build` | |
---
## Phase 6 — Scheduler Integration
**Priority: MEDIUM — after Phases 1–5 pass all tests**
| # | Task | Notes |
|---|------|-------|
| 6-1 | **Pre-Phase-6 audit**: `grep -rn 'eval_select\|alloc_thread\|sched_session' src/scheduler/` — confirm allocator-thread-only invariant | Document result ("confirmed single-threaded" or "found unexpected call site at [location]") in commit message |
| 6-2 | **Blocking gate**: `grep -rn 'sched_meet_host_selection\|meet_host_selection' src/` — all hits must be accounted for before signature change | Missed call site → wrong ABI → UB at runtime (garbage stack parameter) |
| 6-3 | Add `_Atomic int session_active`, `eval_in_progress`, `staging_merge_complete` to `xce_scheduler_t` in `src/scheduler/include/scheduler.h` | Use `_Atomic int` not `volatile int` — ARM atomicity guarantee |
| 6-4 | Implement GC predicate wrappers referencing real `xce_scheduler_t` fields (replace Phase 1 `fake_scheduler_t` stubs) | Signatures unchanged; only wrapper body changes |
| 6-5 | Restructure scheduler session loop to **outer-host / inner-job** pattern with single `select_eval_ctx_t` per session | Current stub returns 1 — full from-scratch implementation |
| 6-6 | Update `sched_meet_host_selection` to `void` return; migrate `add_job_preason(PREASON_MATCH_FAIL)` into inner-job loop | Deleting the preason call silently leaves jobs appearing to hang |
| 6-7 | Implement `build_host_eval_ctx` (scheduler-private in `src/scheduler/src/`) — flatten `worker->resvals` zarray into `numvals`/`strvals` arrays using `Value_u_fvalue`/`Value_u_svalue`/`Value_u_bvalue` from capnp-generated header | Confirm header location: `grep -rn 'Value_u_fvalue' src/scheduler/` |
| 6-8 | Materialize `live_jobs` flat array from `sched->jobs` kvmgr using growth-loop heap allocation (not fragile `zarray_t.data` cast) | |
| 6-9 | Wire `sel_optimize_set(1)` at scheduler startup; `sel_optimize_set(0)` at API server startup | Add `log_fatal` guard at session loop entry if `sel_get_optimize_level() <= 0` |
| 6-10 | Set `sel_alloc_thread_id` at allocator thread first iteration; add thread-ID checks in `eval_select` and `gc_sweep_select_nodes` | Skip check when `sel_alloc_thread_id == 0` (unregistered — Phase 1–5 unit tests) |
| 6-11 | Write integration/parity test against known-good resreq/host pairs | Stub always returns 1, so no meaningful baseline exists — must test explicitly |
---
## Key Implementation Traps
1. **`free_select_tree` vs GC mode confusion** — calling `free_select_tree` in CSE-enabled mode or forgetting it in disabled mode both produce memory corruption. The `sel_get_optimize_level()` guard inside `free_select_tree` catches this with `log_fatal`.
2. **`get_leaf_value()` bypass bug** — calling `get_leaf_value()` directly in comparison branches bypasses the `num_vals`/`str_vals` cache write. Comparisons silently read 0.0/NULL (calloc default) instead of actual host values. Every comparison operator must call `eval_select(child, ctx)` first, then read the typed cache array.
3. **TOCTOU in `makeSelectNode`** — the double-check under lock (step 5) prevents node leaks when two threads race to intern the same node. The re-find must happen inside the lock or the winner could be freed before the loser reads it.
4. **`atomic_thread_fence` before GC** — root-unlink writes (`primary->select->root = NULL`) must be visible to the GC mark pass. Without a seq_cst fence, CPU/compiler reordering causes GC to retain a node that should be swept (or vice versa).
5. **Partial-OOM in `select_eval_ctx_t` resize** — if one of the three reallocs (bool_bits, num_vals, str_vals) succeeds and another fails, the capacities diverge and `eval_select_inner` can write out-of-bounds to `num_vals` (no inline bounds guard at write site). Treat all three as atomic — `log_fatal` on partial failure.