W5-C: exact setcons solver, one count+set optimization, typed CalibrationBundle, internal telemetry, abstention contract - #51
Conversation
… thaw helper in schema, drop dup engine hunks Review (05:19) fixes: - F1: -v/--verbose re-raises engine-path exceptions (full traceback); default stays one clean line + exit 1. Global-flag pre-scan via _last_verbose (dispatch parses args only on success paths). - F2: full multi-line error message printed, not first line only. - F4: CliError deleted (defined, caught, never raised). - F5: engine.py _prior_cache_key + test_engine_fake.py hunks dropped — duplicates PR #51 commits e77a868/034c7c5 which merge first. - F6: serve.py imports engine.load_engine directly again (no cli dependency); _serve_handler split and 'host or 127.0.0.1' fallback removed; test monkeypatches jevmlx.engine.load_engine. - F7: _thaw moved to schema.py as _thaw_plan next to _freeze_plan; evalrun imports it for compiled_plan_sha256. - F3: OSError alone covers the file-error subclasses; tuple slimmed. - F8: validate test dual-path hasattr removed. - F9: gc.get_objects() port scan replaced with patched HTTPServer subclass capturing the bound address. - CHANGELOG Unreleased bullet added (error contract, --json purity, doctor alias, serve bound-port log). Gates: ruff clean; 690 passed (full suite incl. 21 slow real-model).
load_engine returns a frozen Engine dataclass: model, tokenizer, model_id
(resolved), revision, profile, vocab_size, weight_bytes,
cache_capabilities, width_slope — every per-model property resolved ONCE
at load. The prompt profile is probed in load_engine from the RESOLVED
model id (an alias and the full id share the one cached object; is-
identity).
- Generation entry points take an Engine only: run_parallel_generation /
run_parallel_generation_batched / run_naive_generation; callers swept
(api, calibrate, cli, evalrun, parity, serve, bench, benchmark scripts).
- _get_or_compute_prior takes the Engine itself (no second Engine
construction path, no fake per-model properties).
- _prefill takes the engine's profile as a required parameter; the
profile is resolved in load_engine only (_resolve_profile removed).
- width_slope: carried on the Engine; the process global and accessor are
gone. _width_bin_max_rows takes the slope as a parameter.
- Tests build engines through the conftest.make_engine factory; the CLI
smoke suite fakes at the load_engine seam (an Engine in, Engine out).
- New tests: slow-tier load_engine('quality') is load_engine(full-id);
fake-tier proof that run_parallel_generation reads the Engine's carried
properties (vocab/weights/slope) instead of re-introspecting.
- Docs: ARCHITECTURE.md gains the Engine object section + updated module
row; CHANGELOG Unreleased entry.
The prior-cache key hunk is NOT here (owned by PR #51).
Review (2026-09-19) blockers + gaps: - GAP A/F4: ONE Ledger PER CONTEXT in decide_many — each result's flat *_ms keys derive from ITS OWN ledger (own prefill span, own assembly spans, amortized share of the group's merged scoring pass via _amortize_group_spans); group_wall + the merged pass spans live on a per-group ledger. prefill_ms is never the batch-wide sum. - GAP B/F5: Ledger.__exit__ with an exception unwinds only to the FAILING span — ancestors stay open (a Metal retry under group_wall no longer drops the parents); spans nested INSIDE the failing span are dropped. - F1: the unresolved merge conflict in _assemble's docstring resolved. - F2: finalize_public_result dual path deleted (no t0, no else branch); every key comes from derived_flat. - F3: second_pass_ms is the dependency span only (duplicated comment removed); no depends_on => no dependency span (contract 0.0). - F6: _eval_cache_state(b_cache) unconditional again (unpadded merges evaluated; cache_merge spans the whole broadcast region). - F7: batched R==0 branch wraps its assembly in a span and reads the group interval after the span closes, like the main loop. - F8: the transformer span covers mx.eval(out) (real sync inside). - F9: PrefillResult.t_prefill_ms deleted; stale docstrings fixed; dead perf_counter/t0 leftovers deleted; prior_ms from derived_flat. - F10: ONE top-level request span per run_parallel_generation — elapsed_ms is true wall time (derived_flat prefers it). - F11: everywhere except _score_rows' retry paths (documented exception-drop semantics). - F12: Ledger imported once at module top; prior span renamed prior_pass; cache-hit ~0 span noted in docs. - F13: per_item_end_to_end_ms = own prefill span start -> own assembly span end + shared prior_ms (docs and code now agree; PR body amended). Tests: 4 new (per-context prefill_ms differ, second_pass_ms == the dependency span, forced Metal retry under group_wall survives, unpadded cache eval unconditional) + ScoreRowsResult._fields check. 721 passed; slow suite green except the pre-existing api.py KeyError (PR #51).
|
Replied against the 248902e diff as instructed — the push had failed silently; everything is verified against 'gh pr diff 51' now. Head: a0f6aff, rebased over 5449b64 (main). BLOCKERS:
SHOULD: F6-F8 ✅ (infeasible count recorded in count telemetry 'dropped_reason' + logger.warning with field name; dead elif deleted; is_feasible hard cap at 20 options/component raises ValueError, test added). F9 ✅ (single adjacency build; dead branches deleted). NITS: F10 ✅ (PROMPT_VERSION direct), F11 ✅ (typed has_multi), F12 ✅ (scalar telemetry emits calibrated {temperature} + calibration_id; confidence_model kept clean — provenance rides telemetry only), F14-F17 ✅. Full suite green (766 passed incl. slow tier); ruff clean (only pre-existing UP038s on main). |
…per interval jevmlx/engine.py: - ONE Ledger per request in run_parallel_generation and run_parallel_generation_batched; REQUIRED parameter on every stage that records a span (no optional/dual path): _prefill (prefill), _score_rows (cache_merge/transformer/gather), _rescore_rows_batch1 + _make_rescore_evidence_fn + _rescore_multi_options (rescore), score_scalar_field/score_multi_field/_score_all_fields, reconcile_case_constraints (reconciliation), run_dependency_waves -> _selective_second_pass (dependency), _assemble, finalize_public_result. - The neutral prior pass runs in the ledger's 'prior' phase (prior_ms = that span; total_ms includes it). - ScoreRowsResult.gather_ms/broadcast_ms DELETED — the ledger spans are the measurement of record; the internal t_gather_ms/t_broadcast_ms accumulators are gone. A failed forward/gather records NO interval (the ledger drops spans an exception unwinds through). - finalize_public_result derives EVERY flat *_ms key from Ledger.derived_flat() — same keys, no accumulator fallback; second_pass_ms = the dependency span. - Batched: group_wall span per group (prefill + one merged scoring pass + every assembly); per-context assembly spans; per_item_end_to_end_ms = the context's own prefill-span start -> its assembly-span end (honest per-context latency, no fabricated splits); prefill intervals tracked per context index. benchmarks/check_results.py: TIMING_SPLIT_KEYS unchanged; provenance note (ledger-derived). ARCHITECTURE.md: timing.py row (ADOPTED), result-dict flow, timing-split contract row. CHANGELOG Unreleased. Tests: tests/test_w5b14_ledger.py (7) — keys are ledger derivations (total = prior + elapsed, suffix_eval >= lm_head), prior phase, second_pass_ms = dependency span, group wall vs amortized, honest per-item end-to-end, no dual fields on ScoreRowsResult. 697 fast passed; slow suite green except the pre-existing api.py '<field>#count' telemetry KeyError (routed to PR #51 by the coordinator).
Review (2026-09-19) blockers + gaps: - GAP A/F4: ONE Ledger PER CONTEXT in decide_many — each result's flat *_ms keys derive from ITS OWN ledger (own prefill span, own assembly spans, amortized share of the group's merged scoring pass via _amortize_group_spans); group_wall + the merged pass spans live on a per-group ledger. prefill_ms is never the batch-wide sum. - GAP B/F5: Ledger.__exit__ with an exception unwinds only to the FAILING span — ancestors stay open (a Metal retry under group_wall no longer drops the parents); spans nested INSIDE the failing span are dropped. - F1: the unresolved merge conflict in _assemble's docstring resolved. - F2: finalize_public_result dual path deleted (no t0, no else branch); every key comes from derived_flat. - F3: second_pass_ms is the dependency span only (duplicated comment removed); no depends_on => no dependency span (contract 0.0). - F6: _eval_cache_state(b_cache) unconditional again (unpadded merges evaluated; cache_merge spans the whole broadcast region). - F7: batched R==0 branch wraps its assembly in a span and reads the group interval after the span closes, like the main loop. - F8: the transformer span covers mx.eval(out) (real sync inside). - F9: PrefillResult.t_prefill_ms deleted; stale docstrings fixed; dead perf_counter/t0 leftovers deleted; prior_ms from derived_flat. - F10: ONE top-level request span per run_parallel_generation — elapsed_ms is true wall time (derived_flat prefers it). - F11: everywhere except _score_rows' retry paths (documented exception-drop semantics). - F12: Ledger imported once at module top; prior span renamed prior_pass; cache-hit ~0 span noted in docs. - F13: per_item_end_to_end_ms = own prefill span start -> own assembly span end + shared prior_ms (docs and code now agree; PR body amended). Tests: 4 new (per-context prefill_ms differ, second_pass_ms == the dependency span, forced Metal retry under group_wall survives, unpadded cache eval unconditional) + ScoreRowsResult._fields check. 721 passed; slow suite green except the pre-existing api.py KeyError (PR #51).
… leftovers - N2 (crash): the batched R==0 branch no longer opens a nested group_wall and stores results[idx] like the main loop (one store path, one _per_item_end_to_end_ms write); repro added as a test. - N1: prior_ms now exposed on every batched result — derived_flat takes an optional prior_ms override for ledgers with no prior span (the neutral pass runs once on the request ledger, finding 26); total_ms includes it; test added (two contexts, prior_correction=True, prior_ms > 0 on both). - REAL BUG behind N2/round-2: _SpanContext.__exit__ checked identity against the wrong object — every exc-info unwind no-op'd and leaked the span (stack held _OpenSpan, self was the context). _open now returns the pushed _OpenSpan and __exit__ pops by identity against it; ancestors survive (GAP B semantics intact). - F4: _amortize_group_spans DELETED (no fabricated per-context intervals, no _intervals writes) — group spans stay on the group ledger; the amortized share is the ONE derived number per_item_amortized_ms via Ledger.batched_views (N5, now used). - F13: per_item_end_to_end_ms = own prefill start -> own assembly end (the note's definition); prior_ms reported separately; PR body updated. - F9: dead timers deleted (_selective_second_pass t0/second_pass_ms, _build_schema_rows t_plan0/plan_compile_ms). - F11: with-forms for prefill/rescore/reconciliation/dependency (the finally-form recorded failed attempts). - N4: batched ScoreRowsResult passes failed_attempts through; retry test asserts >= 1. second_pass_ms test is now an equality against the dependency interval. - Tests: 14 in test_w5b14_ledger.py (N2 repro, N1 shared prior added). Gates: ruff clean, 733 passed; pre-existing api.py KeyError (#51). - Rebased onto origin/main 5449b64.
|
Follow-up: F6-F9 + F14-F17 now also done — head 7c985b4 (rebased over 5449b64, merge-base 5449b64, fast-forwardable).
|
load_engine returns a frozen Engine dataclass: model, tokenizer, model_id
(resolved), revision, profile, vocab_size, weight_bytes,
cache_capabilities, width_slope — every per-model property resolved ONCE
at load. The prompt profile is probed in load_engine from the RESOLVED
model id (an alias and the full id share the one cached object; is-
identity).
- Generation entry points take an Engine only: run_parallel_generation /
run_parallel_generation_batched / run_naive_generation; callers swept
(api, calibrate, cli, evalrun, parity, serve, bench, benchmark scripts).
- _get_or_compute_prior takes the Engine itself (no second Engine
construction path, no fake per-model properties).
- _prefill takes the engine's profile as a required parameter; the
profile is resolved in load_engine only (_resolve_profile removed).
- width_slope: carried on the Engine; the process global and accessor are
gone. _width_bin_max_rows takes the slope as a parameter.
- Tests build engines through the conftest.make_engine factory; the CLI
smoke suite fakes at the load_engine seam (an Engine in, Engine out).
- New tests: slow-tier load_engine('quality') is load_engine(full-id);
fake-tier proof that run_parallel_generation runs on the Engine's
carried properties (vocab/weights/slope) instead of re-introspecting —
the test calls run_parallel_generation against a model whose true shape
disagrees with the Engine.
- Docs: ARCHITECTURE.md gains the Engine object section + updated module
row; CHANGELOG Unreleased entry.
The prior-cache key hunk is NOT here (owned by PR #51).
load_engine returns a frozen Engine dataclass: model, tokenizer, model_id
(resolved), revision, profile, vocab_size, weight_bytes,
cache_capabilities, width_slope — every per-model property resolved ONCE
at load. The prompt profile is probed in load_engine from the RESOLVED
model id (an alias and the full id share the one cached object; is-
identity).
- Generation entry points take an Engine only: run_parallel_generation /
run_parallel_generation_batched / run_naive_generation; callers swept
(api, calibrate, cli, evalrun, parity, serve, bench, benchmark scripts).
- _get_or_compute_prior takes the Engine itself (no second Engine
construction path, no fake per-model properties).
- _prefill takes the engine's profile as a required parameter; the
profile is resolved in load_engine only (_resolve_profile removed).
- width_slope: carried on the Engine; the process global and accessor are
gone. _width_bin_max_rows takes the slope as a parameter.
- Tests build engines through the conftest.make_engine factory; the CLI
smoke suite fakes at the load_engine seam (an Engine in, Engine out).
- New tests: slow-tier load_engine('quality') is load_engine(full-id);
fake-tier proof that run_parallel_generation runs on the Engine's
carried properties (vocab/weights/slope) instead of re-introspecting —
the test calls run_parallel_generation against a model whose true shape
disagrees with the Engine.
- Docs: ARCHITECTURE.md gains the Engine object section + updated module
row; CHANGELOG Unreleased entry.
The prior-cache key hunk is NOT here (owned by PR #51).
…4-24), review round complete
FINDINGS 14-17 (exact solver):
- setcons.select_constrained_set: connected components over group overlap
AND implication edges; exhaustive bitmask enumeration per component with
implication closure applied per candidate; score-maximizing with the
threshold proposal as tie-break (bit-stable when constraints don't
bind). is_feasible = the same solver, unscored (propagates internal
errors — a solver bug is not infeasibility). Component cap: > 20
options raises (2^n enumeration must not hang the compile).
FINDINGS 18-20 (count + set are ONE optimization):
- The trusted count synthesizes exact_k / at_least_4 and enters the SAME
solver call as the schema's set constraints. Precedence: schema
constraints are HARD, the count is evidence — jointly infeasible drops
the count with dropped_reason in the count telemetry + a warning with
the field name (no silent drop).
- decision_margin is min over ALL options of |p_yes - 0.5| (probability
units), recomputed after EVERY reconciler — an option forced against
its threshold side gets margin 0.
FINDING 24 (internal telemetry):
- Count rows live under internal_telemetry ONLY; a count row in
field_telemetry raises. prior reads internal_telemetry.
FINDINGS 21-23 (calibration contract):
- ONE bundle shape ({model_revision, prompt_version, scoring, prior_mode,
scalar:{temperature}, multi:{a,b}}); CalibrationBundle.load owns file
I/O; the engine takes a CONSTRUCTED bundle only (provenance checked in
the hot path: prompt_version / scoring / prior_mode / temperature
conflicts rejected). Scalar fields emit calibrated {'temperature': T} +
calibration_id when a fitted temperature was applied; confidence_model
stays the clean model name.
- Prompt bumped to jevmlx-parallel-v9 (telemetry keys changed).
Review round: blockers F1-F5, shoulds F6-F9, nits F10-F17 all addressed
(_prior_cache_key stays ids-only — main's version; CLI calibrate writes
the ONE shape with fit-time provenance via engine.model_id). Full suite
green (813 passed incl. slow tier).
Scope (GPT-REVIEW-2 findings 14-25 + C.8/C.10)
Each finding verified in code at 1f9f453 before changing; every new test fails on the base.
Set-cons exact solver (14-16) - jevmlx/setcons.py
Schema compiler (17)
Engine multi block (18-20)
Calibration (21-22)
jevmlx calibrate --outwrites it (provenance captured at fit time), from_payload parses it, no legacy shape. _load_calibration returns (bundle, effective_temperature), validates provenance (wrong prompt_version/scoring/prior_mode/conflicting temperature -> ValueError), resolves ONCE in the batched path and is shared by every _assemble; openai_slots consumes the bundle.API contract (23-25)
Blocking bug fixed on the way (pre-existing on main 649ba70, reproduced on a clean checkout)
_prior_cache_key put weakref.ref objects in the key tuple: hash(ref) delegates to the referent and mlx models are unhashable, so EVERY prior-corrected run crashed with TypeError. Key is id-only now; refs live on the entry with the existing live-ref hit check.
Tests
684 passed, ruff clean. Rebased onto 54cc97b (PR #45).