[REVIEW ONLY — DO NOT MERGE] OrcEngine Phases 1–5B through Stage 2A - #103
Draft
hardcoreerik wants to merge 39 commits into
Draft
[REVIEW ONLY — DO NOT MERGE] OrcEngine Phases 1–5B through Stage 2A#103hardcoreerik wants to merge 39 commits into
hardcoreerik wants to merge 39 commits into
Conversation
…ainst oracle Implements the smallest real C++ OrcEngine inference core, differentially verified against the trusted Phase-0 Python oracle, per the Phase-1 steering document. F32-only, CPU-only, zero external dependencies, C++20/CMake. - Contract types kept distinct per the explicit "don't collapse LogicalTensor into a malloc'd pointer" instruction: TensorShape / LogicalTensor / BackingExtent / ResidentView / ModelManifest / Model / ExecutionPlan / Context, each trivially implemented for Phase 1 but architecturally ready for Phase 6B's paged/streamed residency work. - Every operator in one transformer block: embedding lookup, RMSNorm, linear (x @ W^T), non-interleaved split-half RoPE, GQA causal attention, SwiGLU FFN, tied/untied lm_head resolution, greedy argmax -- written to mirror oracle/ops.py line-for-line for direct reviewability. - Reuses Phase 0's own Fixture C dimensions (vocab=32, hidden=16, intermediate=32, n_layers=2, n_q_heads=4, n_kv_heads=2, head_dim=4) rather than inventing a new mathematical target. - Fixture export (oracle/export_cpp_phase1_fixture.py) to a flat, dependency-free text format -- no JSON library, since both producer and consumer are this repo's own code. - Differential harness (tests/test_gates.cpp) compares all 34 intermediate taps plus final logits plus greedy token selection against tied AND untied fixtures, reporting max_abs_error/max_rel_error/first_bad_index on any divergence. Both fixtures pass every tap at float32 machine-epsilon divergence (~1e-7), four orders of magnitude inside the 1e-3 threshold. Verified the harness actually detects faults (not a rubber stamp) via a deliberately injected weight corruption, correctly localized to only the taps algebra predicts should diverge. - NaN/Inf checked on every tap, fails closed; opt-in ORCENGINE_DEBUG_TAPS=1 tracing. Branch feat/orcengine-phase1, worktree F:/Ai/OrchestratorIDE-phase1, based on feat/orcengine-phase0's tip -- the frozen Phase-0 evidence branch is untouched. PROJECT_TRUTH.md and CURRENT_STATE.yaml updated with precise, evidence-gated Phase-1 status fields (real model loading, GGUF, CUDA, quantization, and TheOrc integration all still explicitly not_implemented). Per the steering document's explicit stop-gate instruction, Phase 1 work pauses here pending maintainer review. Phase 2 has not started. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n, metamorphic tests Answers the four freeze-blocking questions raised before Phase 1 can freeze, each resolved experimentally rather than asserted: - True autoregressive greedy decoding (tests/test_decode.cpp): full-recompute 8-step decode from seed tokens [1,5], C++ and an independently-computed Python trace (oracle/export_cpp_phase1_decode_fixture.py) each choosing their own next token with no knowledge of the other's choice. All 8 steps matched token-for-token: [1,5,5,5,29,29,29,29,29,29]. - F32 vs F64 accumulation A/B, made deliberate rather than accidental: the accumulator type is now configurable (ops::AccumT), with dual CMake targets (orcengine_phase1 = F32 default, orcengine_phase1_f64accum = F64 comparison-only). F64 is marginally more precise but not materially necessary (~5 orders of magnitude inside the 1e-3 threshold either way) -- F32 accumulation is the selected default, matching the stated need for a clean F32 reference ahead of CUDA/quantized comparisons. - Activation representation: confirmed ActivationBuffer (forward.hpp) was already distinct from ResidentView -- ResidentView is used exclusively for durable model weights. Closed the actual gap, which was a missing doc comment stating the distinction explicitly, not a real type collapse. - Metamorphic residency tests (tests/test_metamorphic.cpp): three bit-exact properties proving logical model identity survives physical storage changes -- backing relocation, tied-alias vs physically-separate-but- byte-identical duplicate, and evict/rematerialize. All three pass via exact memcmp, on both accumulation variants. First tiny proof of OrcEngine's oversized-model thesis: same logical model, same result, regardless of physical residency. All 3 test binaries (test_gates, test_decode, test_metamorphic) pass together via ctest. PROJECT_TRUTH.md and CURRENT_STATE.yaml updated with the new evidence; CURRENT_STATE.yaml re-validated as parseable YAML. Phase 1 remains stopped pending maintainer review -- Phase 2 not started. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…R-021 Two documentation-only gaps found during independent Phase-4 pre-review, both fixed before the review proceeds: - CURRENT_STATE.yaml: the id:4 phase entry still read state: not_started while the top-level status.lifecycle string already said Phase 4 was "ready for independent freeze review" -- direct self-contradiction within the same file. Also found id:3 (Phase 3) still read state: implemented_pending_independent_freeze_review despite orcengine-phase3-freeze being an immutable, pushed, verified tag -- Phase 3 is actually complete_frozen. Both corrected, following the existing per-phase schema (completed_date/completion_note for 3, note for 4 citing the new ADR). Re-parsed and validated as YAML after editing (14 phases intact). - DECISION_LOG.md: added OE-ADR-021, the next correctly numbered entry after OE-ADR-020 (scouted via grep before assuming the number). Documents the evidence-driven Phase 3 -> Phase 4 roadmap correction: the original plan (PHASE3_WORKING_SET_SPEC.md section 15) targeted tokenizer/KV-cache/BLAS for Phase 4; Phase-3 freeze measurement showed retained embedding/output bookends were 88.88-94.11% of the remaining weight-residency peak, which motivated bookend/row-region virtualization instead. Original assumption, triggering measurement, experiment, result, and alternatives-considered are all preserved in sequence -- the pivot is recorded as a correction, not rewritten as if it were the predetermined destination. No code changed. No tag moved. Phase 4 remains untagged pending independent freeze review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…WITH FIXES Independent freeze review of feat/orcengine-phase4-bookend-virtualization, attacking Codex's self-review conclusions rather than accepting them, per maintainer instruction. Verified true by direct code inspection and from-scratch reproduction (not just re-reading docs): TensorRowRegion contract is genuinely row-only, no overclaimed slice/tile/quantization-block support; streaming.cpp has zero GGUF references (format-neutral core confirmed); validate_complete_row_partition is a real, non-vacuous partition-completeness check; the weird physical-layout test genuinely exercises logical-vs-physical byte divergence with five fault types; F16 row decoding matches full materialization exactly at six tested positions; the real-artifact corruption check performs a genuine semantic attack on a copied 653MB GGUF; complete-logits comparison (not argmax-only) is real; the residency budget admission gate works as documented. Reproduced: 12/12 deterministic tests (fresh Debug+Release build), 20/20 real-artifact Release campaign (after the same directory-junction workaround Phase 3's own hardening report already documents for a pre-existing environmental issue), 12/12 strict /W4 /WX /permissive- lane, one Grok full review (CLEAN), one Grok adversary review. The adversary Grok pass found a real documentation-precision defect, independently confirmed: the headline 14,162,688-byte peak (2.17%/2.63% of full weights) is conditional on StreamingConfig::output_chunk_rows staying at or below 6,146 rows for this model, not an unconditional property of the row-region strategy -- a caller choosing a larger chunk size would push output-projection residency back toward the old Phase-3 bookend size. All six tested/documented chunk sizes (1-1024) are safely under this threshold, so every specific measured number remains real and reproducible; this is a claims-precision issue, not a code defect (the budget check correctly enforces whatever limit is configured for any chunk size). Fixed in place: PHASE4_BOOKEND_VIRTUALIZATION.md, PROJECT_TRUTH.md, CURRENT_STATE.yaml (re-validated as parseable YAML) now state the peak/ reduction figures as conditional, with the exact threshold shown. Added OE-ADR-022 recording the independent review's method, what was confirmed, what was found wrong, and the ACCEPT WITH FIXES verdict. No engine or test code changed -- no freeze-blocking defect was found in the implementation itself. Phase 4 remains untagged; Phase 5 has not started. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…2_forward
real_forward_check.py's load_real_weights() always read from
convert_real_candidate.py's module-level SOURCE_DIR constant (a path
relative to whichever worktree the Phase-0 oracle package lives in), with no
way to override it -- even though the sibling gguf_real_hf_pytorch_forward/
streaming_real_hf_pytorch tests already correctly accepted an explicit HF
source directory via ORCENGINE_HF_SOURCE_DIR. This meant gguf_real_f32_forward
specifically failed in any worktree whose own artifacts/smollm2-135m/ wasn't
separately populated, even when a valid HF source existed elsewhere and was
already configured for the other tests in the same CMake invocation --
recurring, this session, the same hardcoded-relative-path issue Phase 3's own
hardening report already hit and worked around once with a directory
junction.
Fixed narrowly: _load_config()/load_real_weights() now accept an optional
source_dir override (default preserves the original SOURCE_DIR behavior for
every other caller, e.g. convert()); real_forward_check.py accepts an
optional 4th CLI argument; Phase2's CMakeLists.txt passes
${ORCENGINE_HF_SOURCE_DIR} through to gguf_real_f32_forward when set,
matching the pattern its sibling tests already used. No artifact-management
redesign.
Proven: a 21/21 real-artifact Release CTest run used ORCENGINE_HF_SOURCE_DIR
pointed at a different worktree's artifacts directory
(OrchestratorIDE-phase2-gguf), with no directory junction, symlink, or copy
present -- confirmed absent immediately before the build.
See docs/OrcEngine/DECISION_LOG.md OE-ADR-023 for the full closure record.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The existing "throwing row-region observer is isolated from inference" check installed an observer that threw unconditionally on the very first event StreamingModel::forward emits (ModelExecutionBegin), before any TensorRowRegion* event fires -- so it never actually exercised a throw occurring DURING row-region event handling, despite its name. Added a second, focused check: the observer stays silent until it observes a real TensorRowRegionMaterialized event, throws only there, and explicitly asserts each claim separately -- the event was reached, the observer threw specifically during that event's handling, observer_failure_count == 1, inference still completed, and output remained bit-identical to the frozen reference. All five assertions passed on first run; the original generic-throw test is kept unmodified as its own (still useful) coverage. Test-only change. No engine defect was found or needed fixing. See docs/OrcEngine/DECISION_LOG.md OE-ADR-023 for the full closure record. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reconciles documentation with the two preceding code/test closure commits and records the full closure decision. - PHASE4_BOOKEND_VIRTUALIZATION.md: replaced the ambiguous "13/13"-style validation matrix with a canonical, condition-labeled test-registration table (12 unconditional, +1 with ORCENGINE_FROZEN_PHASE1_SNAPSHOT, up to 21 with all real-artifact options) reproduced experimentally in three configurations (12/12, 13/13, 21/21). Added a "Freeze-hygiene closure" section documenting all three fixes with proof. Added "Region granularity is a policy variable" -- the chunk-size-conditional finding from OE-ADR-022 preserved and generalized into a verified formula derived from forward_impl's actual, confirmed-sequential execution order, explicitly not promoting the model-specific 6,146-row threshold to an engine constant. Reproduction commands updated to configure all four optional CMake variables together. - ENGINEERING_ROADMAP.md: Phase 6B gained one paragraph recording "region granularity is a policy variable, not a fixed floor" as ExecutionPlanner evidence -- no planner work authorized. - DECISION_LOG.md: added OE-ADR-023, the full closure record (root causes, fixes, reproduction proof, alternatives considered for all three items). - PROJECT_TRUTH.md, CURRENT_STATE.yaml: updated with the closure summary; CURRENT_STATE.yaml re-validated as parseable YAML (14 phases intact). Phase 4 remains untagged. Phase 5 has not started. Tagging remains a separate, deliberate maintainer decision. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implements docs/OrcEngine/PHASE5A_KV_CACHE_SPEC.md's bounded hypothesis: does one-token-at-a-time cached decode produce exactly the same logits as full-prefix recompute, at every step, with cache content itself (not just downstream logits) verified correct? - ContiguousAttentionKVStore (Phase 1's previously-unused KV-cache contract type) gains real bounds-checked write_k/write_v/k_row/v_row accessors and an explicit current_length() counter -- its first real exercise since being defined as a contract point in Phase 1. - New Tools/OrcEnginePhase5A/ project: forward_cached_step() mirrors oracle/model.py's already-proven forward_cached() step-for-step (RoPE at absolute cache position, unchanged GQA mapping, rectangular causal mask), reusing Phase 1's ops::* functions and AccumT accumulator directly rather than duplicating transformer math. - New oracle/export_cpp_phase5a_cache_fixture.py exports a prefill + 8-step incremental-decode trace from the trusted Python oracle, including full per-layer KV-cache content after every step, not just logits. Result on synthetic Fixture C: all 9 steps (1 prefill + 8 decode) bit-exact against the independent Python trace (max_abs_diff=0.000000 at every step), exact greedy token selection, cache content independently verified. All 6 required fault-injection cases (wrong cache position, stale/unwritten KV reuse, swapped K/V, corrupted shared GQA head, causal-mask boundary consistency, cache non-leakage between sequences) shown to diverge from the correct reference as required. Two failure-semantics cases (capacity overflow, empty input) fail closed. Debug 8/8, Release 8/8, strict /W4 /WX /permissive- 8/8 (zero warnings), MSVC ASan 8/8 (no findings). One real test-harness bug found and fixed during this pass (not an engine defect): the differential harness's sequence tracker double-counted each decode step's input token, since a decode step's new_tokens is, by the Python export's own construction, already reflected in the NEXT step's seq_before. Caught by the harness's own divergence check. Real SmolLM2-135M cached-decode validation is explicitly deferred (synthetic Fixture C only for this gate, per the spec's own scope). Not yet independently reviewed -- no orcengine-phase5a-freeze tag authorized by this commit. Docs also synced from feat/orcengine-phase4-bookend-virtualization's post-freeze commit (CURRENT_STATE.yaml, DECISION_LOG.md, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md) since this branch forked from the orcengine-phase4-freeze tag, one commit before that roadmap reconciliation; CURRENT_STATE.yaml re-validated as parseable YAML (15 phases) after the Phase 5A results were added to it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Phase-4 gap found Real-model (SmolLM2-135M) KV-cache correctness now proven via a 4-way differential (OrcEngine full/cached vs HF/PyTorch full/native-cached), matching historical Phase 2/3/4 tolerance evidence almost exactly. Real 9Q/3KV GQA, real RoPE-position, real 8192-token capacity-boundary, and cross-context-isolation fault attacks all pass (11/11, test_real_cache_attacks.cpp). Transactional failure semantics proven, not just reasoned about: a mid-step NaN fault leaves the cache poisoned in place while current_length() stays uncommitted, and a retry at the same position safely overwrites the poison and reproduces the untouched baseline bit-exactly (6/6, test_transactional_semantics.cpp). KV memory accounting (46,080 bytes/token, derived independently from the real GGUF's own config) is now empirically confirmed, not just code-inspected: cache allocation produces a measured ~377.5 MiB process working-set jump matching the derived value almost exactly (gguf_cached_forward.cpp's new Psapi-based instrumentation). Most important finding: Phase 5A does NOT compose with Phase 3/4's streaming/row-region virtualization -- it requires a fully-resident Model and never touches ModelSource/TensorRowRegionMaterializer/the Phase-3 layer lifecycle. Correctness is proven; bounded-residency composition is not. Recorded as REJECTED-SUPERSEDED in OE-ADR-025 rather than silently assumed solved. Full validation matrix (Debug/Release/strict-WX/ASan) is 13/13 clean across all four lanes on the current HEAD. Proposed verdict: NOT READY -- BLOCKERS REMAIN (composition gap + no independent review yet). No orcengine-phase5a-freeze tag created, branch not pushed, per standing constraints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lent to Phase 4 (OE-ADR-027) Implements Reference Path C (VirtualizedCachedModel), composing persistent KV state with Phase 3/4's transient, per-layer-materialized weight architecture, per OE-ADR-026's decision. Built entirely on Phase 3/4's existing public ModelSource/TensorMaterializer/ TensorRowRegionMaterializer/ResidencyLedger contracts -- no frozen Phase 1/3/4 file modified. Shared execution seam: execute_cached_transformer_layer() extracted from the prior forward_cached_step (a pure refactor, proven bit-identical before any new code was written) is the only implementation of cached-decode's per-layer math. Both the fully-resident reference (Path B, retained per OE-ADR-026 as a semantic oracle) and the new virtualized path (Path C) call it -- neither duplicates it. Prefill schedule (layer-major vs token-major) proven equivalent on a deterministic fixture before either was chosen; layer-major selected for its real weight-I/O advantage under streaming materialization. Correctness: B == C bit-identical on synthetic Fixture C (9/9 steps) and the real pinned SmolLM2-135M (4/4 steps); A (frozen Phase-4 full-prefix) == B == C bit-identical on the real model too, extended to a full 5-way differential with HF/PyTorch (all five legs agree). Real KV cache content numerically cross-checked against HF's own independently-constructed cache at 5 layer/head/position combinations (max_abs 1e-7 to 2.4e-5). peak_resident_weight_bytes measures at 14,162,688 bytes, matching Phase 4's documented frozen peak exactly. Backing I/O measured, not assumed: transformer weight bytes read are effectively identical between cached and full-prefix execution -- caching does not reduce weight rereads per token under this architecture, while embedding backing bytes are measurably reduced. Composed KV/weight crossover (from Path C's own measured peak): 308 tokens. Commit-API narrowed against the "failed step + set_current_length()" poisoned-commit misuse OE-ADR-025 flagged: forward_cached_step and VirtualizedCachedModel::step now auto-commit on success only. Mid-layer NaN failure re-attacked end-to-end against both reference paths after the refactor. Full fault-attack suite (11/11) re-run against Path C with temporary materialized weights, including a forced-materialization failure and two non-vacuity checks. Full validation matrix (Debug/Release/strict-WX/ASan) 18/18 across all four lanes, zero warnings, zero memory-safety findings. Proposed verdict: READY FOR INDEPENDENT FREEZE REVIEW (19/20 active-gate items satisfied, one partial -- per-step backing-I/O granularity measured at run-level, not fully separated per step). This is a recommendation, not a self-authorization: no orcengine-phase5a-freeze tag created, branch not pushed, per standing constraints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd finite-value parity Closes P5A-RVW-002 (CRITICAL) and P5A-RVW-003 (MAJOR) from the 2026-08-18 independent review. forward_cached_step() and VirtualizedCachedModel::step() now REQUIRE start_position == cache.current_length(), throwing KVCacheError before any mutation otherwise -- decode can no longer silently begin at the wrong committed position. The prior unchecked implementations survive as forward_cached_step_unsafe_explicit_position() / step_unsafe_ explicit_position(), reserved for deliberate fault-injection tests (new test_cache_position_safety.cpp; existing fault-injection cases in test_cached_decode.cpp/test_real_cache_attacks.cpp/test_virtualized_ cache_attacks.cpp updated to call the unsafe seam explicitly). VirtualizedCachedModel::step gained the same check_finite bookend calls (input embedding, final normalized state, logits) that Reference Path B already had, closing an asymmetry where NaN/Inf could silently propagate through Path C but not Path B. materialize_layer now validates each layer has exactly one of each of the 9 required TensorRole values before materializing anything, catching a duplicate-plus-missing-role combination that a bare tensors.size()==9 check could not detect.
…d attack hardening Closes P5A-RVW-004 (CRITICAL), P5A-RVW-005/009/010/011 (MAJOR/MINOR) from the 2026-08-18 independent review. real_5way_composed_differential.py now hard-asserts a_vs_b/a_vs_c/ b_vs_c bit-identity and a direct c_vs_hf_cached comparison (previously computed-but-unused or entirely absent), plus an explicit all_steps_present length gate. The KV-oracle numeric cross-check now requires len(kv_results) == len(dump_specs) before PASS is possible -- a previously-silent skip on an unresolved sample is now a hard error. test_virtualized_cached_decode.cpp replaces the one-sided materialization-count <= check with an EXACT equality against a deterministically computed expected count, closing a gap where under-materialization could not be detected. test_virtualized_cache_attacks.cpp gains attack 11: corrupt only Path B's resident weights and confirm Path C -- independently snapshotted via bind_memory_model before the corruption, so it cannot alias B's live storage -- remains bit-exactly unaffected. This is the mirror image of the pre-existing corrupt-only-C attack, closing the one-directional independence gap.
…n CTest Closes P5A-RVW-006 (CRITICAL) from the 2026-08-18 independent review. New test_real_composed_evidence.cpp asserts A/B/C bit-identity on the real GGUF-backed model (explicit and tied artifacts) inside a compiled, CTest-registered, ASan-covered binary -- previously only the synthetic in-memory materializer exercised real Path C under strict/ ASan. Also includes a real Path-C fault-attack section (wrong-position rejection at the API boundary, corrupted real KV head divergence, RoPE reset-to-0 divergence via the unsafe seam, forced real materialization failure propagating cleanly without committing, capacity-boundary fail-closed) and a real-model prefill-schedule comparison (layer-major batched vs. token-major single-step, bit-identical logits/cache content). CMakeLists.txt: normalized real-artifact configuration to prefer explicit -D ORCENGINE_REAL_F32_GGUF / ORCENGINE_REAL_TIED_F32_GGUF / ORCENGINE_HF_SOURCE_DIR variables (matching Phase 3/4's own convention) with environment-variable fallback; added a redundant find_package(Python3 REQUIRED COMPONENTS Interpreter) call (Phase 3's own Python3_EXECUTABLE is a normal, non-cache variable not visible in this file's own CMakeLists scope via add_subdirectory alone); raised TIMEOUT from 900 to 3600 for the three most expensive real-model tests after ASan showed a ~40-50x slowdown on real-model compute during closure validation.
…dation matrix Documentation reconciliation for the P5A-RVW-001..011 closure pass (OE-ADR-028): DECISION_LOG.md gains the full closure entry plus a clarifying annotation on OE-ADR-027's frozen-file wording; PHASE5A_KV_CACHE_SPEC.md gains a "Freeze-closure pass results" section and updated status banner; PROJECT_TRUTH.md and CURRENT_STATE.yaml reconciled to the same state. Final confirmed validation matrix, all real artifacts configured (explicit + tied GGUF + HF source dir): Debug 30/30, Release 30/30, strict (/W4 /WX /permissive- /EHsc) 30/30 zero warnings, ASan 30/30 zero memory-safety findings (22 via CTest, 8 re-run directly outside CTest's timeout mechanism since their timeouts live in frozen Phase 2/3 CMakeLists.txt files that were not modified). No lane produced an actual correctness or memory-safety failure -- every closure-pass failure encountered was a harness wall-clock budget too small for ASan's real-model slowdown, not a defect. Proposed verdict remains a recommendation, not a self-authorization: READY FOR FINAL INDEPENDENT FREEZE REVIEW, pending a new Grok full+adversary review of this closure HEAD. No freeze tag created, no push performed, no Phase 5B/5C/6/CUDA/product-integration work begun.
…002/003/004/010 for real The re-review (grok_adversary_20260819_092904.md) correctly rejected two "closed" dispositions from the prior closure pass as overstated, and flagged two real coverage gaps: BLOCKER, P5A-RVW-010 (materialization-failure residency): the prior "resolved by manual trace, no leak found" disposition was correct about the absence of a leak but wrong to skip a direct assertion. test_virtualized_cache_attacks.cpp attack 8 now asserts telemetry().current_resident_weight_bytes equals exactly the one permanent FinalNorm bookend after a forced mid-layer materialization failure -- the weight-byte ledger check that peak_active_layers<=1 and current_length()==0 alone cannot provide -- mirroring Phase 3's own test_streaming.cpp "simulated post-release failure leaves clean accounting" precedent. BLOCKER, P5A-RVW-003 (fail-closed parity): the prior closure added Path C's three check_finite bookend calls but no test actually exercised them -- only the per-layer checks SHARED with Path B via execute_cached_transformer_layer were ever attacked. New attacks 12/13/14 inject NaN via a corrupting materializer directly at each bookend (input_embedding via a poisoned token-embedding row, final_normalized_state via a poisoned FinalNorm weight materialized once in the constructor, logits via a poisoned output-projection row -- distinguished from the embedding-row corruption by row_count, since this fixture is tied-embeddings) and confirm step() throws closed at each site specifically, not just generically. MINOR, P5A-RVW-004 (durable A/B/C enforcement): test_real_composed_ evidence.cpp only ever bit-compared the final-token logits slice. Added a full-array comparison at the s==0 prefill step (where Path A's still-2-token forward() and Paths B/C's own 2-token initial cached step cover the identical span), closing the real-model interior- prefill-position gap the synthetic fixture-only equivalence test had already covered but the real model had not. MINOR, P5A-RVW-002 (Path C parity with Path B): test_cache_position_ safety.cpp's Path C section was missing the large-gap-rejects and physical-cache-content-unchanged-on-rejection cases that Path B's section already had. Added both, mirroring Path B's checks exactly. All four fixes verified: rebuilt and re-run in Release (test_virtualized_cache_attacks, test_cache_position_safety, test_real_composed_evidence against the real SmolLM2-135M artifact) -- every new and pre-existing check passes.
The prior OE-ADR-028 entry claimed "MSVC ASan 30/30" as a completed fact before the round-2 re-validation against c89e780 had actually finished -- specifically, the 8 tests whose timeouts live in frozen Phase 2/3 CMakeLists.txt files had not yet been re-run at the time that text was written. That claim is now replaced with the honest split result: CTest itself reports 22/30 passed, 8 timed out, exit code 8 -- that exit code is correct and is NOT characterized as green. The 8 timeouts are a harness wall-clock budget too small for MSVC ASan's 28-40x real-model slowdown, not a correctness defect. Each of the 8 was additionally re-run as the exact same driver-script invocation CTest itself would run, outside CTest's own TIMEOUT mechanism, with exit code, elapsed time, and full output captured: all 8 exited 0, all 8 completed with no AddressSanitizer diagnostic in any captured output, all 8 show their own expected PASS assertion. Combined result: 22 CTest passes + 8 direct-invocation passes, 30/30 tests confirmed with no unproven result -- but the CTest run itself is never described as passing, since its own exit code is legitimately non-zero.
… evidence Gate item 18 still cited the pre-closure 18/18 count as the live status. Updated to the actual final numbers for this closure pass: Debug/Release/strict 30/30 each (strict zero warnings); ASan CTest 22/30 with eight inherited timeouts and exit code 8 (inherited from frozen Phase 2/3 CMakeLists.txt TIMEOUT properties, not modified this pass), those same eight exact invocations confirmed by direct invocation with exit 0 and no ASan diagnostics. The CTest exit code is not characterized as passing. Docs-only, single-line change. All historical 18/18 references elsewhere in this document (the pre-closure OE-ADR-026/027 baseline) are left untouched, since they correctly describe an earlier point in time before the suite grew to 30 tests.
…bdirectory
Confirmed by the 2026-08-19 adversary freeze review: the ENV{}
fallback for ORCENGINE_REAL_F32_GGUF / ORCENGINE_REAL_TIED_F32_GGUF /
ORCENGINE_HF_SOURCE_DIR ran AFTER add_subdirectory(Phase 3), which
pulls in Phase 2. Phase 2's own CMakeLists.txt unconditionally
declares these same three names as CACHE variables with an empty
default. Since a CACHE set() without FORCE is a no-op once the entry
already exists, Phase 2's empty-default declaration always won the
race -- the NOT DEFINED check here was always false (DEFINED does not
distinguish an empty cache entry from a meaningfully configured one),
so the environment fallback could never fire. This silently dropped
real_cache_attacks and every inherited Phase 2/3 real-artifact test
whenever only the environment variable, not -D, was set -- the exact
opposite of the "backward compatibility... retained" claim already in
this file's own comments.
Fix: move the three fallback blocks before add_subdirectory, so an
explicit -D (which creates its cache entry before any CMakeLists.txt
runs) is already DEFINED here and wins; the environment variable is
used only when no -D was given; Phase 2's own empty-default
declaration becomes the no-op once either path has already populated
the entry.
Validated (configure-only, no build/execute, per this session's
bounded scope):
- Fresh build configured with ONLY the three environment variables
(no -D): `ctest -N -C Release` registers all 30 tests, including
real_cache_attacks and every inherited Phase 2/3 real-artifact test
(streaming_real_*, gguf_real_*).
- Fresh build configured with a conflicting environment value AND an
explicit -D: CMakeCache.txt confirms the -D value wins.
Does not modify any frozen Phase 1/2/3/4 file -- this file is Phase
5A's own CMakeLists.txt.
…view Resolves the 1 BLOCKER and 3 FIX-BEFORE-FREEZE findings from the Grok full+adversary freeze review run against af2dc59 (.orc/reviews/grok_full_20260819_213426.md, grok_adversary_20260819_213800.md). The 2 OPTIONAL findings (gate item 18's word choice; real-GGUF resident-byte assertion coverage) are recorded as deferred, not fixed with new test code, per this closure round's explicit scope. CURRENT_STATE.yaml (BLOCKER): top-level fields still described synthetic-only Phase 5A, real-model validation as unfinished, and a stale active_worktree_head -- directly contradicting the reconciled phase5a_status sub-block below them, which gate item 20 claims this file is reconciled with. Replaced the static active_worktree_head (falsified the instant its own updating commit lands; no consumer found) with last_independently_reviewed_head: af2dc59, and updated phase5a_kv_cache_status / phase5a_real_model_validation / phase5a_fault_injection_cases_passing (86 check() assertions, grep-derived from the four attack test files, not invented) to match the actual closure-pass evidence. Added a Phase 5A paragraph to the top-level summary, which previously never mentioned Phase 5A at all. PHASE5A_KV_CACHE_SPEC.md / DECISION_LOG.md / PROJECT_TRUTH.md (FIX BEFORE FREEZE): P5A-RVW-010's disposition still said "resolved by manual trace, not additional code" in all three files, stale since c89e780 added attack 8d's direct current_resident_weight_bytes assertion. Corrected to record the code assertion as the closing evidence, trace as supporting evidence only, with the real-GGUF-path coverage gap explicitly noted as optional strengthening (not a new gap, since the exercised code path is shared with the already-covered synthetic path). PHASE5A_KV_CACHE_SPEC.md (FIX BEFORE FREEZE): P5A-RVW-003's CLOSED text claimed NaN-injection coverage via "the same real-model and synthetic test infrastructure" -- only synthetic attacks 12-14 exist. Corrected to state the fail-closed checks are implemented in the shared virtualized path and directly attacked synthetically; real- model numerical execution is covered separately, but real-model NaN injection was not performed. PHASE5A_KV_CACHE_SPEC.md (OPTIONAL, addressed anyway since it was low-cost alongside the BLOCKER fix): gate item 18's criterion literally read "all pass" while the same bullet's evidence disclosed ASan CTest at 22/30. Reworded the criterion to require zero correctness/memory- safety failures with any CTest harness exception documented, and reframed the ASan evidence as complete underlying validation with a documented CTest harness exception -- not a green CTest run. Validated: YAML re-parses clean; grep for "resolved by trace" and "real-model and synthetic" across all three truth documents returns no matches; git diff --check clean.
Small refinements on top of the already-committed closure (7f52493 CMake fix, fd01aba doc reconciliation), addressing the remaining precision items from a re-issued closure spec: - CURRENT_STATE.yaml as_of.date was still 2026-08-18, one day stale relative to the commits it describes. - lifecycle field said "...phase_5a_in_progress", which understates the actual state (closure pass complete, awaiting maintainer freeze approval, not still being worked on) -- reworded to make that distinction explicit. - The phase5a_status sub-block's active_gate_status/proposed_verdict/ independent_review fields still said "pending new independent review" -- stale since that review (Grok full+adversary against af2dc59) has since completed, its findings resolved, and a focused diff review confirmed clean. Updated to reflect that without claiming a freeze tag, push, or self-authorized freeze exists (none do). - PHASE5A_KV_CACHE_SPEC.md gate item 18's status marker was plain "SATISFIED", which invites the same "all pass" reading the criterion wording itself was already corrected to avoid. Changed to "SATISFIED WITH DOCUMENTED CTEST HARNESS EXCEPTION" so the marker itself, not just the prose after it, carries the nuance. Validated: fresh env-only CMake configure registers all 30 tests (ctest -N -C Release); fresh -D-vs-conflicting-ENV configure confirms -D wins via CMakeCache.txt; CURRENT_STATE.yaml re-parses clean; grep for stale "resolved by trace"/"real-model and synthetic"/"ASan... green" phrasing across all four truth documents returns no matches; git diff --check clean; focused Grok diff review of this exact uncommitted change: CLEAN.
The last active documentation contradiction: CURRENT_STATE.yaml already truthfully records that Phase 5A's closure pass is complete, that the required full+adversarial independent review ran against af2dc59, that its confirmed findings were resolved, and that focused reviews of those corrections came back clean -- but this file's active (non-historical) wording still said "READY FOR INDEPENDENT FREEZE REVIEW", "MAINTAINER/INDEPENDENT REVIEW STILL REQUIRED", "pending re-review", and "READY FOR FINAL INDEPENDENT FREEZE REVIEW", falsely implying the independent review had not yet happened. This made gate item 20's "documentation reconciled" claim false. Four active locations corrected to a consistent READY FOR MAINTAINER FREEZE APPROVAL vocabulary that distinguishes "independent review completed" from "maintainer approval, freeze tagging, and push still pending": the top-level Status block, the binding active-gate verdict (items 18-20), the "Freeze-closure pass results" final disposition paragraph, and the "Independent-review requirement" section (new disposition note appended, general policy preserved). Item 15's partial-satisfaction and its bounded-gap framing are unchanged. One historical verdict (line ~498, an earlier REJECTED-SUPERSEDED-labeled snapshot predating the composition work, inside a section the document's own reading-order note already marks superseded) was left untouched, per this project's append-only documentation discipline. No code, CMake, YAML, tests, fixtures, build directories, or other document touched -- this is the single remaining documentation correction from the 2026-08-19 freeze-review closure sequence. Validated: rg search for "pending re-review", "independent review still required", "ready for final independent", "ready for independent freeze review" returns no matches; git diff --check clean.
Maintainer explicitly approved OrcEngine Phase 5A for formal local freeze on 2026-08-20. This commit records that acceptance across the authoritative documents before the orcengine-phase5a-freeze tag is created at this commit. CURRENT_STATE.yaml: as_of.date to 2026-08-20; lifecycle, phase5a_kv_ cache_status, and the phase5a_status sub-block's active_gate_status/ proposed_verdict/freeze_tag updated to reflect the freeze (freeze_tag: orcengine-phase5a-freeze, replacing "absent"); added phase5a_freeze_tag/ phase5a_freeze_verdict/phase5b_status fields following the existing phase1-4_freeze_tag convention (freeze commit hash intentionally omitted -- same self-reference problem as the removed active_worktree_head field; the tag is the durable authority, verified via `git rev-list` after creation, not asserted here); updated the Phase 5A summary paragraph. Gate item 15's bounded partial and the optional real-GGUF ledger assertion remain visible and unchanged. PROJECT_TRUTH.md: OE-ADR-027/028's own dated verdict paragraphs preserved verbatim as historical, each with a one-line cross-reference added (not a rewrite) pointing to the current status; new "OE-ADR-029: Phase 5A formal maintainer freeze, 2026-08-20" section added as the current Phase 5A truth; "Current blockers" rewritten to state none remain for Phase 5A. PHASE5A_KV_CACHE_SPEC.md: top-level Status block, the binding 20-item gate verdict, the "Freeze-closure pass results" final disposition, the "Independent-review requirement" disposition, and the Stop gate all updated from READY FOR MAINTAINER FREEZE APPROVAL to ACCEPTED AND FROZEN BY MAINTAINER (2026-08-20, orcengine-phase5a-freeze). The Stop gate no longer prohibits tag creation outright -- it now states the tag was created by the authorized freeze procedure and that Phase 5B, if later authorized, must begin as a separately scoped effort, not further work on this frozen line. One genuinely historical NOT READY verdict (an earlier REJECTED-SUPERSEDED-labeled snapshot predating the composition work) was left untouched per this project's append-only discipline. DECISION_LOG.md: appended OE-ADR-029 recording the maintainer's explicit approval, the frozen parent, the freeze tag (target verified post-creation, not asserted here), the three reference paths and their roles, the 30-test validation matrix with the honest ASan 22-CTest + 8-direct split (never described as green), independent full+adversarial review and focused correction-review status, gate item 15's accepted bounded partial, the optional real-GGUF ledger assertion explicitly recorded as deferred (not implemented), what remains unauthorized (push/merge/PR/5B/etc.), and Phase 5A's immutability after tagging. Added one minimal cross-reference line to OE-ADR-028's own acceptance- trigger paragraph (preserved otherwise unedited) pointing to this entry. ENGINEERING_ROADMAP.md: added a concise frozen-status note under Phase 5A (freeze date, freeze tag, Phase 5B/5C dependency ordering). Phase 5B's own description untouched -- still "not started; spec to be written when 5A closes," not expanded. Validated: CURRENT_STATE.yaml re-parses clean; git diff --check clean; diff limited to exactly these five files (confirmed via git diff --name-only); search for the nine specified stale/current-language phrases found 3 matches, all classified -- one accurately-phrased current text (past-tense "was the sole remaining blocker", correctly describing resolution), one explicitly self-labeled historical note already redirecting to the current section, and one vocabulary enumeration immediately followed by the actual verdict declaration -- none required further correction.
New docs/OrcEngine/PHASE5B_TOKENIZER_SPEC.md (status: DRAFT FOR MAINTAINER REVIEW -- NO IMPLEMENTATION AUTHORIZED). Specification-only pass, no implementation, per the maintainer's explicit authorization scope for this task. Bounded hypothesis: for the single pinned SmolLM2-135M tokenizer profile, a native C++ text/token boundary consuming the required GGUF tokenizer metadata can produce token IDs and decoded byte behavior that exactly agree with independent trusted oracles, without changing the frozen OrcEngine tensor-execution core. Compatibility tuple confirmed by this pass's own targeted reconnaissance (not carried over from documentation alone): GPT-2-style byte-level BPE (tokenizer_class: GPT2Tokenizer, model.type: BPE), GGUF tokenizer.ggml.model="gpt2"/pre="smollm", vocab 49,152 / merges 48,900 (confirmed matching between tokenizer.json and the real GGUF), 17 special/control tokens (token_type=3) vs. 49,135 ordinary tokens (token_type=1), BOS=EOS=UNK all token ID 0 (<|endoftext|>), add_bos_token=add_eos_token=false, add_prefix_space=false, no GGUF unknown_token_id field and no BYTE/UNKNOWN token_type entries anywhere in the vocabulary (a derived architectural fact: this byte-level vocabulary has no practical unknown-token encode path). Confirmed no native tokenizer implementation exists anywhere in the C++ tree; the existing Phase 2 GGUF metadata reader (gguf.hpp, already supporting array-typed values) is identified as the correct reuse target for a future implementation, not something to duplicate. 10-item decision register recording default-BOS/EOS (metadata- determined, no maintainer decision needed), skip_special_tokens default, invalid-UTF-8 handling, and 5 other genuinely open policy questions -- each with existing evidence, a recommended contract, and an explicit yes/no on whether maintainer confirmation is still required. Nothing is silently decided where local evidence doesn't determine it. Reuses rather than recreates: the 20-fixture tokenizer golden corpus, the raw-prompt-identity fixtures, the dual-source tokenizer agreement evidence, and the tokenizer_special_token_error fault-injection case -- explicitly noting none of these have ever exercised a native OrcEngine implementation, since none exists yet. Minimal state-document updates to prevent contradiction: CURRENT_STATE.yaml (repository/branch now point at this Phase 5B worktree; phase5a_base_authority added; phase5b_status updated from not_started_not_yet_specified to specification_drafting_started, distinct from implementation_not_started); ENGINEERING_ROADMAP.md (Phase 5B section links the new spec, heading updated); PROJECT_TRUTH.md (two statements that would have become false -- "Phase 5B has not started and is not yet specified" in two places -- corrected to distinguish specification drafting from implementation). DECISION_LOG.md intentionally not touched: drafting a proposed specification is not an accepted architectural decision. Validated: CURRENT_STATE.yaml re-parses clean; all new relative doc links resolve; search for 5 specified contradictory-language patterns across docs/OrcEngine/ returns zero matches; git diff --check clean; diff limited to exactly these four files (git diff --name-only); no source, test, CMake, artifact, fixture, or frozen Phase 1-5A file changed; Phase 5A worktree (F:\Ai\OrchestratorIDE-phase5a-kv-cache) confirmed untouched and clean throughout.
Focused correction pass on PHASE5B_TOKENIZER_SPEC.md, no other file touched. Every fix below is backed by newly gathered, cited local evidence, not by argument alone. 1. Frozen-file contradiction fixed: Section 4 no longer says a future implementation "must extend gguf.hpp" (a frozen Phase 2 file). Direct inspection confirms GgufArtifact::metadata (public field), require_metadata() (public function), and GgufValueType::Array (already supporting nested arrays) already expose everything this profile needs -- a Phase-5B-local free function is sufficient. No frozen-file exception is required or requested. 2. Fixture-evidence classifications corrected by reading the actual committed artifact (tokenizer_golden_fixtures.json) instead of trusting the category list: empty_input -> [], round-trips exactly; embedded_nul_char -> [17985, 190, 9110], round-trips exactly; text_resembling_special_tokens encodes <|endoftext|> as ID 0; text_resembling_special_tokens_2 encodes <|im_start|>/<|im_end|> as IDs 1/2; both show the skip_special_tokens divergence directly; add_special_tokens true/false was confirmed to NOT change the plain bos_eos_combination fixture. 3. Encode-policy/oracle mismatch resolved with a targeted local investigation (tokenizers 0.22.2, exact commands/results recorded in Section 7): add_special_tokens does NOT control literal special-token recognition despite the name (confirmed empirically, identical IDs both ways). The actual controlling property is Tokenizer.encode_special_tokens (default False): False is the oracle's own default and matches literal control-token substrings to their special IDs (Mode B); True suppresses that and encodes them as ordinary byte-level text (Mode A). Both modes are now confirmed independently producible against the primary oracle. The secondary oracle (llama.cpp) could not be checked this pass -- no binary found locally, ORC_LLAMA_TOKENIZE_PATH unset -- recorded as an evidence gap, not invented behavior. 4. Frozen-engine integration proof corrected: [1, 5, 28, 284, 260, 198] is [1, 5] (arbitrary explicit IDs, never established as a tokenization) plus [28, 284, 260, 198] (generated, not tokenized). No raw-prompt-identity fixture produces [1, 5] -- checked all six directly. Section 11 now uses the established "Hello, world!" -> [19556, 28, 905, 17] fixture instead, and no longer requires the tokenizer to produce generated continuation IDs. 5. Byte-representability claim narrowed from "every possible input byte sequence is representable" to "every valid UTF-8 input... is representable... without a vocabulary-level unknown-token fallback" -- invalid UTF-8 is now explicitly a separate input-validation decision (Decision Register item 6), not conflated with vocabulary coverage. 6-7. Decision Register recount and Maintainer decision packet: the register already had seven "Yes" rows (1, 2, 5, 6, 7, 8, 10), not six as previously summarized -- corrected everywhere referenced. New Section 19 gives each of the seven a decision/evidence/recommendation/ consequence-both-ways/evidence-sufficiency writeup; none are marked silently accepted. 8. No changes to CURRENT_STATE.yaml, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md, or DECISION_LOG.md -- this pass did not create a contradiction requiring one, and no maintainer decision was accepted (drafting/correcting a proposed specification is not an ADR). Validated: git diff --name-only shows exactly this one file changed; git diff --check clean; contradictory-language search (phase 5b not started / no spec / implementation started / accepted / frozen) returns zero matches; every referenced local file path confirmed to exist in this worktree (the two tokenizer.json/tokenizer_config.json mentions are descriptive references to the pinned model's own files in the separate Phase2-gguf worktree, per this task's explicit instruction not to copy that directory here -- not broken links).
Maintainer explicitly approved all seven previously-unresolved policy decisions in PHASE5B_TOKENIZER_SPEC.md Section 19 on 2026-08-20: 1. Mode A (literal/ordinary-text encoding) is the default for ordinary user text. 2. Mode B (control-token recognition) requires explicit caller opt-in. 3. Decode preserves special-token text by default; stripping is an explicit option. 4. Invalid UTF-8 input is rejected with an explicit error. 5. An incomplete UTF-8 sequence at end-of-stream is an explicit error. 6. Invalid or out-of-vocabulary token IDs are rejected explicitly. 7. Raw decoded bytes are the primary round-trip correctness authority; Unicode comparison is secondary for valid UTF-8. PHASE5B_TOKENIZER_SPEC.md: status changed from "DRAFT FOR MAINTAINER REVIEW -- NO IMPLEMENTATION AUTHORIZED" to "SPECIFICATION ACCEPTED FOR IMPLEMENTATION -- IMPLEMENTATION NOT STARTED". Decision Register (Section 18) and Maintainer decision packet (Section 19) both updated with explicit APPROVED 2026-08-20 markers, preserving all existing evidence/consequence text rather than replacing it. Section 14 (Acceptance criteria), Section 17 (Stop gate) clarified: specification acceptance is not implementation-completion or freeze -- those remain governed by Section 14's criteria, none of which can be evaluated before implementation exists, and the outstanding llama.cpp secondary- oracle comparison remains required before Phase 5B can be considered complete or frozen. DECISION_LOG.md: appended OE-ADR-030 (next available number after OE-ADR-029), recording the acceptance decision, the seven approved policies (summarized, full detail left in the specification), the Phase 5A frozen base authority, the specification-acceptance vs. implementation-completion distinction, and the llama.cpp gap classified exactly as instructed: not a blocker to beginning implementation, a required dependency before completion/freeze, not yet satisfied, not evidence of disagreement, and no permission granted to remove that gate. No historical ADR (022-029) modified. CURRENT_STATE.yaml, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md: minimal updates to statements that would otherwise contradict the acceptance (status fields, one summary paragraph each) -- none repeat the full seven-item packet, all point to PHASE5B_TOKENIZER_SPEC.md/OE-ADR-030 for detail. Validated: CURRENT_STATE.yaml re-parses clean; current-language audit (no implementation authorized / drafting started / not started / complete / frozen) returns zero stale matches; all cross-referenced documents confirmed present; git diff --name-only shows exactly these five files; git diff --check clean; no source, test, CMake, fixture, artifact, or frozen Phase 1-5A file touched.
Phase 5B Stage 1: native GGUF tokenizer-metadata construction and
fail-closed validation, per the accepted PHASE5B_TOKENIZER_SPEC.md and
DECISION_LOG.md OE-ADR-030. First implementation authorization under
the accepted specification.
New Tools/OrcEnginePhase5B/ (does not modify any frozen Phase 1-5A
file): TokenizerProfile::from_gguf_metadata(const GgufArtifact&)
constructs an immutable tokenizer profile (vocabulary, merge table,
per-token types, BOS/EOS/add-token configuration) from already-parsed
GGUF metadata, validating every invariant in the spec's Section 8
before returning any object. Reuses the frozen Phase 2 GGUF reader's
existing public surface (GgufArtifact::metadata, require_metadata,
GgufValueType::Array) without modifying it -- confirmed sufficient by
direct inspection, exactly as the spec's Section 4 concluded. No
existing native tokenizer implementation was found anywhere in the
tree to duplicate.
Does NOT implement: text encoding, BPE merge execution,
pretokenization, token decoding, streaming UTF-8 accumulation,
frozen-engine integration, or anything from Stage 2+. Stage 1
constructs and validates tables only.
Validates exactly the pinned compatibility tuple: tokenizer.ggml.
model="gpt2"/pre="smollm", tokens/token_type both exactly 49,152
entries, merges exactly 48,900, token types restricted to {1 NORMAL,
3 CONTROL} with exactly 17 CONTROL tokens at exactly IDs 0-16, bos/eos
both exactly ID 0, add_bos_token/add_eos_token both exactly false.
Fail-closed for every category the spec requires: missing/wrong-typed
fields, unsupported model/pre values, wrong counts, out-of-range or
non-pinned BOS/EOS, malformed/duplicate/unresolvable merges, duplicate
or empty vocabulary entries.
Test (test_tokenizer_metadata, 53 checks: 15 positive/structural + 38
adversarial) constructs a synthetic GgufArtifact directly in memory
(GgufArtifact::metadata is public, GgufValue is a plain aggregate) --
no binary GGUF fixture, no Phase 2 test-helper changes, no JSON
dependency. The synthetic profile matches the pinned tuple's exact
counts (49,152/48,900/17) using deterministic placeholder strings, so
the exact-count invariants are genuinely exercised without needing the
real vocabulary. Proves: valid construction succeeds; exact ordering
is preserved; the public API is read-only (no mutator exists); every
malformed case fails explicitly with no partial result (structurally
guaranteed -- the factory returns by value only on the final success
path); repeated construction from identical metadata is deterministic;
no static/global state or tensor-weight/KV-cache type is referenced
anywhere in the construction path (verified by code inspection, not
runtime instrumentation).
Targeted validation only (no inherited Phase 1-5A test suites run):
Debug, Release, strict (/W4 /WX /permissive- /EHsc, zero warnings), and
ASan (test binary run directly, zero diagnostics) -- all four lanes,
53/53 checks passing in each.
Documentation updated to reflect Stage 1 completion without claiming
Phase 5B is complete, validated as a finished implementation, or
frozen: PHASE5B_TOKENIZER_SPEC.md, CURRENT_STATE.yaml,
ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md. No new ADR -- Stage 1
followed the architecture OE-ADR-030's accepted specification already
described, without a new architectural decision to record. The
llama.cpp secondary-oracle comparison remains an outstanding
validation dependency.
Closure-correction pass on Phase 5B Stage 1, before Stage 2 begins. Not Stage 2 authorization -- no pretokenization/BPE/encode/decode/ streaming/integration code added. Correction 1 (real pinned GGUF acceptance): test_tokenizer_metadata now accepts two optional argv paths (explicit, tied) and exercises the real, committed load_tokenizer_profile(path) entry point against the actual pinned artifacts, not just synthetic in-memory metadata. CMake registers tokenizer_metadata_real conditionally via the SAME ORCENGINE_REAL_F32_GGUF/ORCENGINE_REAL_TIED_F32_GGUF cache variables Phase 2 already declares -- no machine-specific paths in source/CMake, no artifacts copied/modified/regenerated. The zero-argument synthetic invocation is unchanged and still requires no large artifacts. REAL FINDING, not a Phase 5B defect: smollm2-135m-tied.gguf has ZERO tokenizer.ggml.* metadata keys (15 architecture-only fields vs. the explicit artifact's 24 -- confirmed by direct inspection). Correctly rejected by load_tokenizer_profile with a clear GgufError. The explicit-vs-tied identity comparison this correction pass requires is reported as an honest, understood, FAILING check (not silently skipped, not fabricated as a pass, and the test no longer crashes uncaught the way an earlier draft of this fix did). The explicit artifact fully validates. This is a new, separate blocker from the llama.cpp secondary-oracle gap, pending a maintainer decision (e.g. reconverting the tied artifact in a future pass) -- no ADR added, since this is an artifact-completeness fact, not an architectural decision. Correction 2 (complete merge validation): production code now also validates that each merge's CONCATENATED result exists in the vocabulary, confirmed empirically against BOTH real pinned GGUF artifacts before being enforced (48,900/48,900 real merges resolve via simple left+right concatenation, zero exceptions -- no representation nuance, no invented transformation needed). The synthetic "valid" fixture was rebuilt as a genuinely BPE-consistent two-tier vocabulary (235 base tokens x their pairwise concatenations = 48,900 merge-result tokens, exactly filling the 49,135-normal-token budget) so it satisfies the same rule the real tokenizer does, rather than retaining placeholder merges whose concatenations never resolve. New adversarial case: left and right both individually exist, syntax and uniqueness are valid, but the concatenation is absent -- fails with a diagnostic naming the merge index and the missing result. Correction 3 (exception type and diagnostic verification): the adversarial helper no longer accepts "any std::exception". Each of the 41 cases now asserts the SPECIFIC exception type (GgufError from the reused Phase 2 accessors, or TokenizerMetadataError from Phase 5B's own validation) and that the diagnostic contains a stable fragment identifying the rejected field/invariant -- not exact-string equality (brittle), a stable substring. Correction 4 (tautology removal): the two unconditional check(true, ...) runtime passes for "no frozen static state mutated" and "no tensor/KV-cache dependency" are removed as executable checks -- both are code-inspection findings, now recorded as [INSPECTION] log lines and source comments, not counted in the pass/fail tally. Added static_asserts verifying the public accessors genuinely return const references (a real, compiler-checked property). Honest new check count: 136 (synthetic-only) / 149 (with both real GGUF paths). Correction 5 (documentation): fixed the stray "was originally written during**:" markdown fragment; PHASE5B_TOKENIZER_SPEC.md, CURRENT_STATE.yaml, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md updated to the actual honest check counts and the new tied-GGUF finding. No new ADR. Targeted validation only (no inherited Phase 1-5A suites): Debug, Release, strict (/W4 /WX /permissive- /EHsc, confirmed present in CMakeCache.txt, zero warnings), and ASan (test binary run directly; only informational C5072/LNK4302 compiler/linker warnings at build time, zero AddressSanitizer runtime diagnostics) -- all four lanes identical: synthetic 136/136 clean; real-artifact runs 147/149 with the same two understood, non-code failures in every lane.
Green-lane closure pass on Phase 5B Stage 1, resolving the artifact- classification question the prior closure-correction commit (4b20e43) surfaced but did not have authority to settle. Not Stage 2 authorization -- no encoding/decoding/pretokenization/BPE-execution/ streaming/integration code added. Maintainer decision, recorded as DECISION_LOG.md OE-ADR-031: smollm2-135m.gguf is the canonical, positive, tokenizer-bearing Phase 5B artifact. smollm2-135m-tied.gguf is a frozen legacy tensor/ output-head-equivalence fixture with no tokenizer.ggml.* metadata -- it is NOT a positive tokenizer-bearing artifact, and its rejection by load_tokenizer_profile is now a REQUIRED, passing test outcome, not a temporary gap. Explicit-vs-tied tokenizer-table equality -- introduced by the prior closure-correction prompt, never part of the specification accepted at commit 8308314 -- is removed as a Phase 5B requirement. Neither real GGUF artifact is modified or regenerated. Test contract split into three independently satisfiable programs-in- one, each returning exit 0 only when its own checks pass: test_tokenizer_metadata (unchanged: synthetic suite, 136 checks) test_tokenizer_metadata --real-explicit <path> (new: canonical positive suite) test_tokenizer_metadata --expect-missing-tokenizer <path> (new: legacy tied rejection suite) The rejection contract is NOT CTest WILL_FAIL (which would treat any nonzero exit -- a crash, an unrelated exception, a wrong-type rejection -- as a false pass). It explicitly requires: index_gguf succeeds, TokenizerProfile construction throws exactly GgufError (not TokenizerMetadataError, not any other type, not a silent success), and the diagnostic names the missing tokenizer.ggml.model key. CMake registers tokenizer_metadata_real_explicit and tokenizer_metadata_legacy_tied_rejection independently, each gated on its own ORCENGINE_REAL_F32_GGUF / ORCENGINE_REAL_TIED_F32_GGUF cache variable -- neither test requires the other's artifact variable to be configured, and each is reported REGISTERED/SKIPPED separately in configure output. No machine-specific paths committed. tokenizer.cpp's merge-result-validation comment corrected: it previously claimed the concatenation rule was confirmed against "both real pinned GGUF artifacts", which is impossible since the tied artifact carries no merge/tokenizer metadata at all. Now states accurately that all 48,900 merges were confirmed against the canonical explicit GGUF only, with the tied artifact tested separately for fail-closed rejection. The validation logic itself is unchanged -- no new defect was found. Documentation (PHASE5B_TOKENIZER_SPEC.md, CURRENT_STATE.yaml, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md) reconciled: Stage 1 synthetic suite, explicit positive suite, and legacy rejection suite are all described as clean; no registered Phase 5B test intentionally fails; the historical discovery that the tied fixture lacks tokenizer metadata is preserved, not erased. OE-ADR-030 was not rewritten (DECISION_LOG.md change is a pure append, verified via diff). Encoding/decoding remain unimplemented, Phase 5B remains incomplete/unfrozen, the llama.cpp secondary-oracle comparison remains outstanding, Phase 5C remains deferred. Targeted validation only (no inherited Phase 1-5A suites): Debug, Release, strict (/W4 /WX /permissive- /EHsc, confirmed present in CMakeCache.txt, zero warnings), and targeted ASan (binary run directly with the ASan runtime DLL on PATH; only informational C4530/LNK4300 compiler/linker messages, zero AddressSanitizer runtime diagnostics) -- all four lanes: synthetic 136/136, explicit-positive suite full pass, legacy-tied-rejection suite full pass, all three CTest targets exit 0.
Phase 5B Stage 2A: exact native reproduction of the pinned SmolLM2
tokenizer's Digits(individual_digits=true) -> ByteLevel(add_prefix_
space=false, trim_offsets=true, use_regex=true) pretokenization
sequence, producing pretoken BYTE RANGE boundaries only. Does NOT
implement byte-to-Unicode alphabet remapping, BPE merge execution,
token-ID production, decoding, streaming, or frozen-engine
integration -- all separate, unauthorized future stages. Not Stage 2B
authorization.
Approved approach: a compact, generated, immutable Unicode codepoint-
range table, subject to empirical oracle proof. No new runtime
dependency (no ICU/PCRE2/Oniguruma/Boost.Regex/utf8proc) was added.
Classification predicates established and validated against the live
tokenizers==0.22.2 oracle, not assumed from any library's tables:
\p{L} = Unicode General Category {Lu,Ll,Lt,Lm,Lo} (677 ranges)
\p{N} = Unicode General Category {Nd,Nl,No} (144 ranges)
\s = the fixed Unicode White_Space=Y property (25 codepoints)
\s is deliberately NOT Python's str.isspace(): isspace() incorrectly
includes U+001C-U+001F (a CPython-specific historical carve-out) that
this oracle's regex engine does not follow -- caught by direct oracle
probing, not assumed. The Digits(individual_digits=true) stage's
numeric predicate was confirmed IDENTICAL to \p{N} (Nd union Nl union
No, all scripts) after a systematic re-check disproved an earlier,
in-session misreading that had concluded it was ASCII-digit-only; the
real distinction is that Digits isolates each N-class codepoint ahead
of ByteLevel, so ByteLevel's own grouping quantifier never observes
more than one at a time. Digits-stage segment boundaries were
confirmed (via direct oracle probes, e.g. "a 5b") to be a hard stop
for ByteLevel's scan.
Validation before promotion: every one of the 677 L-range and 144
N-range boundaries, plus both neighbors of all 10 fixed S-ranges,
checked against the live oracle (2,831 boundary probes, 0 mismatches
after the str.isspace() correction above); a further seeded
(seed=20260820) random sample of 4,974 codepoints found 0 additional
mismatches; the Digits-stage predicate was separately boundary-
validated against the shared N table (565 probes, 0 mismatches).
Proof: a 63-entry oracle-derived fixture corpus (golden fixtures,
raw-prompt-identity fixtures, and a hand-authored boundary/transition
corpus -- ASCII, contractions, digit runs across several scripts,
CJK, emoji, combining marks, embedded NUL, special-token lookalikes,
category-transition pairs) computed from the real tokenizers==0.22.2
pretokenizer via Tools/OrcEnginePhase5B/tools/generate_pretok_tables.py
(dev-only, not part of the C++ build). test_pretokenize compares the
native scanner byte-for-byte against every entry, plus 8 invalid-UTF-8
rejection cases (never CTest WILL_FAIL -- each case asserts the
rejection is exactly PretokenizeError), plus determinism and
structural checks: 209/209 pass, 0 failures. This is boundary
agreement on that corpus and sampling, not a claim of exhaustive
equivalence over all possible Unicode strings.
Targeted validation only (no inherited Phase 1-5A suites): Debug,
Release, strict (/W4 /WX /permissive- /EHsc, confirmed present in the
actual cl.exe compile commands, zero real warnings), and ASan
(/fsanitize=address /EHsc, confirmed present in the actual compile
commands, C4530 absent, zero AddressSanitizer runtime diagnostics) --
all four lanes: 209/209 pretokenize checks pass, and all three
existing Stage 1 metadata contracts (tokenizer_metadata,
tokenizer_metadata_real_explicit, tokenizer_metadata_legacy_tied_
rejection) remain green, unmodified.
Recorded as DECISION_LOG.md OE-ADR-032. Phase 5B remains incomplete
and unfrozen; the llama.cpp secondary-oracle comparison remains
outstanding; Phase 5C remains deferred.
Closes a Codex review finding against the Stage 2A generator (OE-ADR-032): no defect in the production C++ scanner, but the generator did not fail closed on the oracle identity and was not location-independent. 1. Verify the installed tokenizers package itself (import + compare __version__ == "0.22.2"), aborting before any generation or write if it differs -- never trusts a hard-coded version label. 2. Remove the hard-coded machine-specific tokenizer.json path. Add a required --tokenizer-json argument; resolve every committed repository input/output path from Path(__file__).resolve().parent, not the caller's cwd. No configuration framework added. 3. Verify tokenizer identity before generation: compute the supplied tokenizer.json's SHA-256 and compare against a pinned expected value from the accepted SmolLM2 artifact, aborting on mismatch; then parse and verify the exact required contract (normalizer null, Sequence pretokenizer, Digits(individual_digits=true), ByteLevel(add_prefix_ space=false, trim_offsets=true, use_regex=true)) -- never inferred from the path or filename alone. 4. Replace eval(raw_repr, ...) with ast.literal_eval(raw_repr). 5. Add a read-only --check mode: generates all three outputs in memory, compares byte-for-byte against the committed headers, exits nonzero on drift, never writes in this mode. 6. Complete provenance record added to DECISION_LOG.md OE-ADR-033: Python 3.14.3, unicodedata 16.0.0, verified tokenizers==0.22.2, tokenizer.json sha256, generator sha256, and all three generated- header sha256 values -- none embedded in the files they describe. 7. Tightened evidence wording across PHASE5B_TOKENIZER_SPEC.md, CURRENT_STATE.yaml, ENGINEERING_ROADMAP.md, PROJECT_TRUTH.md to: "Matches the pinned oracle across the 63-entry corpus, all generated category boundaries, and the recorded seeded sample; exhaustive equivalence over every possible Unicode string is not claimed." Validated: --check passes (exit 0, no file changes) identically from the repository root and from the generator's own directory; supplying a monkeypatched wrong tokenizers version, and supplying an unrelated committed JSON file as --tokenizer-json, both abort (exit 1) before any generation. The two generated headers that changed (pretok_tables.hpp, pretok_oracle_fixtures.hpp) changed only in provenance/comment text -- no table range or fixture data value changed. test_pretokenize was rebuilt in Debug against the regenerated headers as a precaution: 209/209 checks pass, 0 failures, confirming no behavioral drift. No Phase 1-5A file changed; no frozen file touched; nothing pushed, tagged, or merged; Stage 2B not started.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hardcoreerik
added a commit
that referenced
this pull request
Aug 23, 2026
…view, PR #103) embedding_lookup had NO validation at all: a negative or out-of-vocab token ID indexed table[token*hidden + h] straight past the end of the backing vector -- an out-of-bounds heap read, not merely a wrong answer. The non-cached forward() path happens to be protected by validate_forward_inputs() calling this first, but that guard is not guaranteed to run before every caller (the cached-decode path added in a later phase does not currently call it either -- see that phase's own worktree for the matching fix). Added: hidden > 0 and table.size() % hidden == 0 preconditions, then a per-token [0, vocab) bounds check derived from table.size()/hidden (no signature change -- vocab isn't a separate parameter). Five new hostile-input regression cases in test_regressions.cpp (negative, off-by-one at vocab, far out of range, non-positive hidden, and a confirming case that valid boundary tokens 0 and vocab-1 still work). orcengine-phase1-freeze unchanged (new commit only). Debug 23/23, strict (/W4 /WX /permissive-) zero warnings, 7/7.
hardcoreerik
added a commit
that referenced
this pull request
Aug 23, 2026
…(Gemini review, PR #103) forward_cached_step_unsafe_explicit_position validated sequence length, start_position, and cache shape, but never checked that token IDs were within [0, vocab) before calling ops::embedding_lookup -- which itself had no bounds check either. A negative or out-of-vocab token ID would index table[token*hidden+h] straight past the end of the weight buffer: an out-of-bounds heap read / potential crash, not merely a wrong answer. The frozen non-cached forward() path is protected by validate_forward_inputs(); this cached-decode path (added in this same phase) never called it or an equivalent. Two-layer fix: ops::embedding_lookup itself now validates hidden>0 and a per-token [0, vocab) bound derived from table.size()/hidden (ported from the same fix already applied on feat/orcengine-phase1, this worktree carries its own copy of Phase 1's files) as the last line of defense for every caller; forward_cached_step_unsafe_explicit_position also gets its own explicit, clearly-attributed check before any per-layer work begins, matching this project's fail-closed-before- mutation discipline. Three new regression cases in test_cached_decode.cpp (negative token, off-by-one at vocab, far out of range) -- all rejected cleanly. Full existing suite (18/18 Debug, including the real-model 91s fault-attack test) and strict (/W4 /WX /permissive-, zero warnings) unaffected. orcengine-phase5a-freeze unchanged (new commit only).
hardcoreerik
added a commit
that referenced
this pull request
Aug 23, 2026
…6 worktree (Gemini review, PR #103) Same fix as feat/orcengine-phase1's and feat/orcengine-phase5a-kv-cache's own commits, ported here since this worktree carries its own copies of those files (forked before the fix existed) and Phase 6 implementation builds on top of this tree: - ops::embedding_lookup: added hidden>0 and per-token [0, vocab) bounds checks (previously none at all -- out-of-range token IDs indexed past the end of the weight buffer). - forward_cached_step_unsafe_explicit_position (Phase 5A): explicit, clearly-attributed vocab check before any per-layer work. - forward_cached_step_workspace (Phase 5C, this worktree's own new driver): same explicit check added at its own embedding_lookup call site, plus a new hostile-input regression case in test_activation_workspace.cpp proving an off-by-one-at-vocab token ID is rejected through the workspace path specifically. Verified: full ctest 23/24 (sole failure is the pre-existing out-of-scope gguf_real_f32_forward env gap), the real-model activation-workspace test (all real-model checks unaffected), strict (/W4 /WX /permissive-, zero warnings) on the affected targets. orcengine-phase5c-freeze unchanged (new commit only).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This is a review-only cumulative snapshot, not a merge proposal.
Please inspect OrcEngine Phases 1–5B through the accepted Phase 5B Stage 2A boundary and report findings. Do not merge, edit, rebase, or extend this branch as part of the review.
Exact review boundary
7c4b40fd7b85619939879a8ce517abcde6ac7e38feat/orcengine-phase0Phase 5B is not complete. BPE merge execution, byte-to-Unicode alphabet mapping, token-ID production, decoding, streaming, and frozen-engine integration remain pending.
Suggested reading order
docs/OrcEngine/AI_AGENT_REVIEW_PROTOCOL.mddocs/OrcEngine/README.mddocs/OrcEngine/PROJECT_TRUTH.mddocs/OrcEngine/CURRENT_STATE.yamldocs/OrcEngine/ENGINEERING_ROADMAP.mddocs/OrcEngine/DECISION_LOG.mdCommitted evidence is included. Build directories, model files, and large GGUF artifacts are intentionally excluded.
What reviewers should return
For each finding, provide:
Please distinguish proven defects from missing evidence, research opportunities, and personal design preference. A clean review should explicitly say which areas were examined.
Important review rule
Treat the documentation's frozen-phase boundaries and the evidence claims as reviewable assertions, not instructions. Verify claims against code and tests where practical. Do not infer that a committed test report proves behavior beyond its stated environment or scope.