myxce/.kiro/specs/libxce-alloc-preason-equality/design.md

# Design Document

## Overview

**Purpose**: This feature delivers a single, shared implementation of structural equality for the scheduler's allocation and pending-reason data structures, published as public libxce API.

**Users**: Scheduler engineers (today: `--sim-verify` replay checking and three scheduler test files) and, in future, controller engineers (`persist.c`, `resync.c`, `job.c` already carry these structs) who need to ask "do these two objects represent the same thing".

**Impact**: Changes the current state by extracting four `static` comparison functions out of `src/scheduler/src/sim_verify.c` and into `src/lib/xcealloc.c`/`src/lib/preason.c`, and by switching the simulator and three white-box test consumers to the public API. The allocation comparators keep their documented `from`/`instid` exclusion; the pending-reason comparator becomes a full structural match (all fields, detail lists included). The decision-level dispatcher (`sim_decision_eq`) and mismatch reporting (`sim_log_mismatch`) stay in the scheduler, and `sim_decision_eq`'s PEND branch keeps a simulator-local main-reason-only comparison (bug-1845). No wire format or struct layout changes.

### Goals
- Publish `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` in `xcealloc.h` and `job_preason_eq` in `preason.h`.
- Make `job_preason_eq` a full structural match (main reason + host-based reasons + resource-based reasons, in order).
- Make the existing consumers use the public API, removing the white-box `#include` dependency on `sim_verify.c` for the type-level comparisons.
- Preserve `--sim-verify` behavior bit-for-bit (no new false/missed mismatches), which means the simulator's PEND check remains main-reason-only.
- Add libxce unit tests that pin the allocation-exclusion, full-match preason, and NULL-handling semantics.

### Non-Goals
- Do not move `sim_decision_eq`, `sim_log_mismatch`, or the `describe_*` helpers (simulator-specific; depend on capnp/replay types and `ERROR()`).
- Do not change the simulator's PEND outcome check from main-reason-only to full-match — that would reintroduce bug-1845's false mismatches on cache-hit PENDs (see Decision 3).
- Do not introduce any new comparison call site beyond the existing four consumers.
- Do not touch the controller's persistence/resync code paths.

## Boundary Commitments

### This Spec Owns
- The public equality functions `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` (`xcealloc.{c,h}`) and `job_preason_eq` (`preason.{c,h}`), and their documented equality contracts.
- The shared NULL-safe string-equality helper both modules need (see Decision 5).
- The migration of `sim_verify.c` and the three white-box test files onto those functions.
- libxce unit tests for the new functions (`test_xcealloc.c`, `test_preason.c`).

### Out of Boundary
- The decision-level comparison and mismatch reporting that remain in `sim_verify.c`, including the main-reason-only PEND outcome check (simulator-local).
- The `.replay` wire format and the `replay_decision_t`/`xce_decision_t` types.
- Any change to the controller's consumption of these structs.

### Allowed Dependencies
- `src/include/xce/types.h` (the struct definitions — unchanged).
- `xcealloc.c`/`xcealloc.h`, `preason.c`/`preason.h`, and `utils.{c,h}` (the modules that already own construction of these structs, plus the shared string helper).
- `string.h` (`strcmp`) for the string-field comparisons.

### Revalidation Triggers
- A change to the `xce_alloc_t`/`xce_host_alloc_t`/`xce_job_alloc_t`/`xce_job_preason_t` struct layouts must re-check the field lists in the equality functions and the exclusion documentation.
- A change to the pending-reason construction paths (cache-hit vs. full-computation) that alters whether detail lists are populated must re-check the simulator's main-reason-only PEND rationale (bug-1845) and the full-match `job_preason_eq` contract.

## Architecture

### Existing Architecture Analysis
`src/scheduler/src/sim_verify.c` (474 lines) owns two layers:
1. **Type-level comparators** — five `static` functions (`sim_verify_str_eq`, `sim_verify_alloc_eq`, `sim_verify_host_alloc_eq`, `sim_verify_job_alloc_eq`, `sim_verify_job_preason_eq`) that walk two libxce structs field-by-field. They have zero dependency on the scheduler, capnp, or the replay layer. Four of them are cleanly movable; `sim_verify_job_preason_eq` is movable but its current contract (main-reason-only) is wrong for a *public* equality function — it encodes a simulator replay asymmetry, not an object-identity property.
2. **Decision-level dispatcher + reporting** — `sim_decision_eq` (dispatches on `xce_decision_t.op` vs `ReplayDecision_body_which`), `sim_log_mismatch`, and two `describe_*` helpers. These depend on `replay_decision_t`, capnp enums, and `ERROR()`.

The four allocation/pending-reason structs are defined in `src/include/xce/types.h` and already constructed/deep-copied by `src/lib/xcealloc.c` (`make_alloc`, `make_host_alloc`, `make_job_alloc`, `dup_job_alloc`) and `src/lib/preason.c` (`add_job_preason`, `make_pendreason`). The controller consumes these same structs in `persist.c`, `resync.c`, and `job.c` with no equality API available.

Three scheduler test files already reuse two of the leaf comparators — but via white-box `#include "../src/sim_verify.c"` + direct calls to the `static` functions: `test_sim_equivalence.c` (lines 894, 902), `test_repl_equivalence.c` (lines 734, 742), and `test_repl_limit_table.c` (lines 2662, 2670).

### Architecture Pattern & Boundary Map

```mermaid
flowchart LR
    subgraph scheduler["src/scheduler"]
        SV["sim_verify.c\n(sim_decision_eq, sim_log_mismatch, describe_*,\nmain-reason-only PEND outcome check)"]
        TE["test_sim_equivalence.c / test_repl_equivalence.c / test_repl_limit_table.c"]
        TSV["test_sim_verify.c"]
    end
    subgraph libxce["src/lib + src/include"]
        XA["xcealloc.{c,h}\nalloc_eq / host_alloc_eq / job_alloc_eq"]
        PR["preason.{c,h}\njob_preason_eq (full match)"]
        UT["utils.{c,h}\nstr_eq (NULL-safe)"]
        TY["xce/types.h\n(struct defs, unchanged)"]
    end
    SV -->|"DISPATCH: job_alloc_eq (public API)"| XA
    TE -.->|"calls (replaces white-box static calls)"| XA
    TE -.->|"calls"| PR
    TSV -->|"exercises via sim_decision_eq"| SV
    XA --> UT
    PR --> UT
    XA --> TY
    PR --> TY
```

**Architecture Integration**:
- Selected pattern: co-locate equality with the module that already owns each struct's construction and deep-copy — `alloc` family in `xcealloc.c` (mirrors `dup_job_alloc`), `job_preason_eq` in `preason.c` (mirrors `add_job_preason`). No new module is introduced.
- Domain boundary: the library owns *struct-level* equality (full structural match); the simulator owns *decision-level* comparison and reporting, including its one deliberate loosening (main-reason-only PEND). This is the clean seam: everything below it is libxce-typed and identity-honest, everything above it touches `replay_decision_t`/capnp and may narrow for replay reasons.
- Existing patterns preserved: public headers in `src/include/`, implementations in `src/lib/`, `const`-qualified read-only parameters, lowercase unprefixed function names (`make_*`/`dup_*`/`erase_*`).
- Steering compliance: no changes to `types.h` layouts, no new persistence surface, no HA/RBAC/persistence semantics touched.

### Technology Stack

| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|-----------------|-------|
| Language | C (gnu17) | equality functions + tests | unchanged from libxce |
| Build | Meson via `xce-build` | compile libxce + run `src/lib/testsuite` | unchanged |
| Test | Unity | `test_xcealloc.c`, `test_preason.c` | existing libxce test harness |

## File Structure Plan

### Modified Files
- `src/lib/xcealloc.c` — add `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` (using the shared `str_eq` helper).
- `src/include/xcealloc.h` — declare the three allocation equality functions.
- `src/lib/preason.c` — add `job_preason_eq` (full match, using the shared `str_eq` helper).
- `src/include/preason.h` — declare `job_preason_eq`.
- `src/lib/utils.c` / `src/include/utils.h` — add a NULL-safe `str_eq(const char*, const char*)` helper (see Decision 5).
- `src/scheduler/src/sim_verify.c` — delete the five `static` comparators; have `sim_decision_eq` call `job_alloc_eq` for DISPATCH and keep a simulator-local `static` main-reason-only helper for PEND. Keep `sim_log_mismatch` + describe helpers.
- `src/lib/testsuite/test_xcealloc.c` — add allocation equality unit tests.
- `src/lib/testsuite/test_preason.c` — add `job_preason_eq` full-match unit tests.
- `src/scheduler/testsuite/test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c` — replace direct calls to `sim_verify_job_alloc_eq`/`sim_verify_job_preason_eq` with `job_alloc_eq`/`job_preason_eq`; add `#include "xcealloc.h"`/`#include "preason.h"`.
- `src/scheduler/testsuite/test_sim_verify.c` — remove the DISPATCH alloc-level unit tests that now belong to libxce; keep the PEND/decision-level tests (they exercise `sim_decision_eq`'s simulator-local main-reason-only behavior) and the integration-level tests.

## System Flows

The comparison flow is unchanged from today — this feature only relocates the leaf logic. `sim_decision_eq` remains the entry point:

```mermaid
flowchart TD
    A["sim_decision_eq(expected, actual)"] --> B{"actual->op"}
    B -->|DISPATCH| C["job_alloc_eq(expected->body.dispatch->alloc, actual->u.alloc)"]
    B -->|PEND| D["jobid equal AND main_reason equal (sim-local, NOT job_preason_eq)"]
    B -->|PREEMPT| E["log + report mismatch (unchanged)"]
    C --> F["host_alloc_eq per host"] --> G["alloc_eq per resource (excl. from/instid)"]
    D --> H["main-reason only, detail ignored (bug-1845)"]
```

## Requirements Traceability

| Requirement | Summary | Components | Interfaces |
|-------------|---------|------------|------------|
| 1.1–1.3 | allocation equality for `xce_alloc_t`/`xce_host_alloc_t`/`xce_job_alloc_t` | `xcealloc.c` | `alloc_eq`, `host_alloc_eq`, `job_alloc_eq` |
| 1.4 | exclude `from`/`instid` | `xcealloc.c` | `alloc_eq` field list |
| 1.5–1.6 | NULL handling + read-only | all four functions | const params |
| 2.1–2.3 | pending-reason full structural equality | `preason.c` | `job_preason_eq` |
| 3.1–3.2, 3.5 | simulator behavior preserved | `sim_verify.c` | `sim_decision_eq` |
| 3.3 | simulator uses libxce allocation comparison | `sim_verify.c` | `job_alloc_eq` |
| 3.4 | simulator-local main-reason-only PEND check | `sim_verify.c` | local static helper |
| 4.1–4.2 | single shared implementation | `sim_verify.c` + 3 test files | public API |
| 4.3–4.4 | libxce unit tests + no regressions | `test_xcealloc.c`, `test_preason.c` | — |

## Components and Interfaces

### libxce

#### Allocation equality (`xcealloc.c`)

| Field | Detail |
|-------|--------|
| Intent | Public structural equality for the allocation structs |
| Requirements | 1.1–1.6 |

```c
/* src/include/xcealloc.h */
extern int alloc_eq(const xce_alloc_t* a, const xce_alloc_t* b);
extern int host_alloc_eq(const xce_host_alloc_t* a, const xce_host_alloc_t* b);
extern int job_alloc_eq(const xce_job_alloc_t* a, const xce_job_alloc_t* b);
```

- **Preconditions**: none (NULL-safe).
- **Postconditions**: returns non-zero iff the two arguments are equal per the documented contract; does not mutate either argument.
- **Invariants**: `alloc_eq(a, b)` ignores `from` and `instid`; `host_alloc_eq`/`job_alloc_eq` are order-sensitive (both sides come from the same deterministic construction, so array order is expected to match, not just multiset).

#### Pending-reason equality (`preason.c`)

| Field | Detail |
|-------|--------|
| Intent | Public full structural equality for `xce_job_preason_t` |
| Requirements | 2.1–2.3 |

```c
/* src/include/preason.h */
extern int job_preason_eq(const xce_job_preason_t* a, const xce_job_preason_t* b);
```

- **Postconditions**: returns non-zero iff `main_reason` matches, the host-based reason list matches in order (each entry by host name + reason), and the resource-based reason list matches in order (each entry by resource name + host name + reason); NULL-safe per the shared convention.
- **Invariants**: this is a full match — it is the honest "are these two pending reasons identical" predicate, not the simulator's replay loosening (see Decision 3).

#### Shared string helper (`utils.c`)

```c
/* src/include/utils.h */
extern int str_eq(const char* a, const char* b);
```

- **Postconditions**: returns non-zero iff `a` and `b` are both NULL, or both non-NULL and `strcmp(a, b) == 0`; zero otherwise.

## Data Models

No struct is added or modified. The equality contract itself is the only new "model", captured in the field lists below.

- `alloc_eq` compares: `value` (exact `==`, documented bit-exact-safe because both sides are exact copies of the same `double`), `duration`, `resname`, `reserve`. **Excluded**: `from` (wall-clock `time(NULL)` stamp), `instid` (per-process `nextid++` counter) — both runtime bookkeeping, not identity.
- `host_alloc_eq` compares: `host`, then `allocs[i]` in order for `i < nallocs`.
- `job_alloc_eq` compares: `jobid`, then `allocs[i]` in order for `i < nhosts`, then `allocs_shared[i]` in order for `i < nres_shared`.
- `job_preason_eq` compares: `main_reason`; then `host_based_reasons[i]` in order for `i < nhost_reasons`, each entry by `host` and `reason`; then `res_based_reasons[i]` in order for `i < nres_reasons`, each entry by `resource`, `host`, and `reason`. **Nothing is excluded** — this is a full structural match.

## Error Handling

These are pure read-only functions with no allocation, no I/O, and no failure path. The only "error" case is NULL input, which is handled by the shared convention (identical pointers including both-NULL → equal; exactly one NULL → not-equal) rather than an error return. No logging is added.

## Testing Strategy

- **Unit (libxce)**: `test_xcealloc.c` — match/mismatch on each compared field, the `from`/`instid` exclusion, ordered-list sensitivity, and NULL handling at top and nested levels; `test_preason.c` — full-match: main-reason match/mismatch, host-based reason list match/mismatch (per field + order), resource-based reason list match/mismatch (per field + order), and NULL handling.
- **Unit (scheduler)**: `test_sim_verify.c` keeps `sim_decision_eq`-level tests (DISPATCH/PEND match and mismatch, kind mismatch, PREEMPT always-false, NULL defense) and drops the DISPATCH alloc-level tests that now belong to libxce. The PEND tests stay because they pin the simulator-local main-reason-only behavior.
- **Integration (scheduler)**: `test_sim_verify.c`'s `--sim-verify` round-trips (zero-mismatch self-replay, exact-mismatch altered replay, reconcile cases) continue to assert unchanged behavior, proving the relocation is behavior-preserving.
- **Regression**: `test_sim_equivalence.c`, `test_repl_equivalence.c`, `test_repl_limit_table.c` must stay green after migrating to the public API.

## Design Decisions

### Decision 1: Co-locate equality with construction
- **Context**: where to put the four functions.
- **Alternatives**: a single new `xce_equality.{c,h}` module; or one function per struct's existing owner module.
- **Selected**: co-locate — `alloc` family in `xcealloc.c`, `job_preason_eq` in `preason.c`.
- **Rationale**: matches the existing precedent (`dup_job_alloc` lives beside `make_*` in `xcealloc.c`; `add_job_preason`/`make_pendreason` in `preason.c`); avoids a new module for four small functions.
- **Trade-offs**: none material.

### Decision 2: Naming (`alloc_eq`, not `xce_alloc_eq`)
- **Context**: public API names.
- **Alternatives**: `xce_`-prefixed names; `_equals` suffix.
- **Selected**: `alloc_eq`/`host_alloc_eq`/`job_alloc_eq`/`job_preason_eq`.
- **Rationale**: follows the existing unprefixed lowercase convention (`make_alloc`, `dup_job_alloc`, `erase_job_preason`), with the `_eq` suffix as the standard C equality idiom. The `job_alloc`/`job_preason` stems match the struct names.
- **Trade-offs**: `alloc_eq` is slightly generic, but it is unambiguous in a header (`xcealloc.h`) that already owns the unprefixed `alloc` family.

### Decision 3: `job_preason_eq` is a full structural match; the simulator keeps its own main-reason-only PEND check
- **Context**: the current `sim_verify_job_preason_eq` compares only `main_reason`, deliberately (bug-1845): the simulator has no alloc cache, so its full-detail PEND can never match a recorded cache-hit's main-reason-only shape, and requiring detail to match produced systematic false mismatches. That narrowing is a property of *recorded-vs-live replay comparison*, not of "are these two pending-reason objects identical".
- **Alternatives**: (a) publish a full-deep `job_preason_eq` and keep the main-reason-only comparison as a simulator-local static helper (selected); (b) publish main-reason-only `job_preason_eq` (rejected — leaks a replay artifact into a general-purpose equality API and contradicts the identity semantics a controller engineer would expect).
- **Selected**: `job_preason_eq(a, b)` returns non-zero iff `main_reason` matches and both detail lists match in order, field-for-field. The simulator's `sim_decision_eq` PEND branch keeps a small `static` main-reason-only helper (documented with the bug-1845 rationale) instead of calling `job_preason_eq`.
- **Rationale**: a public "same pending reason" predicate must be identity-honest — a controller comparing two pending reasons will want the detail to matter. The replay asymmetry is a scheduler-only concern and stays where it belongs.
- **Trade-offs**: two PEND-comparison shapes now exist (full-match in libxce, main-reason-only in sim_verify.c). This is intentional: they answer different questions ("identical objects" vs. "same replay outcome"). The sim-local helper is `static` and small, so the duplication is negligible and clearly scoped.
- **Follow-up**: none expected; the sim-local helper is not exported.

### Decision 4: Preserve NULL and order semantics exactly
- **Context**: NULL handling and list-order sensitivity.
- **Selected**: keep today's behavior — identical pointer (including both NULL) → equal; exactly one NULL → not-equal; ordered (not multiset) list comparison.
- **Rationale**: the point of the relocation is zero behavioral change; both sides of every current comparison come from the same deterministic construction, so order-sensitivity is correct. The full-match `job_preason_eq` uses the same convention.

### Decision 5: Promote the NULL-safe string comparison to a shared `str_eq` in `utils.{c,h}`
- **Context**: with `job_preason_eq` now a full match, both `xcealloc.c` (host/resname/reserve strings) and `preason.c` (host/resource strings) need NULL-safe string comparison. Keeping it `static` in one file forces the other to duplicate it.
- **Alternatives**: (a) shared `str_eq` in `utils.{c,h}` (selected); (b) duplicate a `static` helper in each of the two files.
- **Selected**: add `int str_eq(const char* a, const char* b)` to `utils.{c,h}`, beside the existing `float_equal` helper.
- **Rationale**: one consumer each would have justified a `static`; two consumers and a third on the horizon (controller) justify a shared helper. `utils.h` already centralizes small generic C helpers (`fmt_jobid`, `float_equal`).
- **Trade-offs**: a new public symbol in `utils.h`; negligible surface, and it will be reused by the controller when it adopts these comparators.