refactor: shrink the codebase by ~12.5k source lines with no features lost - #46
Jackson57279 wants to merge 25 commits into
Conversation
|
Warning Insufficient credits for auto-review. Keep at least $0.00 of available balance to start a run. Please add credits to continue. |
|
Important Review skippedToo many files! This PR contains 264 files, which is 164 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (264)
You can disable this status message by setting the 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 |
807e0d3 to
36d7cb8
Compare
|
@cubic-dev-ai review |
@Jackson57279 I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 121,190 of the 120,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 3 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
Jackson57279
changed the title
|
Warning Insufficient credits for auto-review. Keep at least $0.00 of available balance to start a run. Please add credits to continue. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d86fd5df90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| use crate::tensor::{ | ||
| GemmError, GemvError, RmsNormError, gemm_f32, gemv_f32_transposed, rms_norm_f32, | ||
| }; | ||
| use crate::tensor::{GemvError, RmsNormError, gemv_f32_transposed, rms_norm_f32}; |
There was a problem hiding this comment.
When a downstream crate imports oxidize_core::cpu_kernels::CpuWorkspace, FusedRmsNormGemv, fused_rms_norm_gemv_f32_transposed, or matmul_reuse_workspace, this change makes that crate stop compiling even though cpu_kernels remains a public module. The same commit also removes public functions from oxidize_core::tensor, including gemm_i8, gemm_i4, and gemv_quantized_f32_transposed; absence of in-workspace callers does not make exported library APIs dead, so retain/deprecate these symbols or explicitly treat this as a breaking release.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified: oxidize-core is not published (crates.io returns 404 for it; the workspace is a private monorepo at 0.1.0), so there are no downstream importers of cpu_kernels::CpuWorkspace / FusedRmsNormGemv / matmul_reuse_workspace or the tensor helpers. I checked callers across every crate in the workspace before each removal, and the deleted functions had zero non-test callers — several were already #[allow(dead_code)]. Keeping an unwired 6th MoE router and a duplicate quantized-transposed GEMV family is exactly the maintenance surface this PR removes. If this crate ever gets published, the API surface to expose would be a deliberate 1.0 decision, not the current accidental one.
| if (setjmp(oc_test_abort_jmp) == 0) | ||
| t->fn(); | ||
| oc_test_can_skip = 0; | ||
| _exit(oc_test_failed ? 1 : 0); |
There was a problem hiding this comment.
When cr_skip_test runs—for example, an AVX-512 test on an AVX2-only host—it longjmps back here with oc_test_failed == 0, after which the child exits with status 0 and the parent records RESULT_PASS; the managed path can therefore never produce status 77/RESULT_SKIP. On the inspected host, oxk_ext/parity_q8_0_scalar_vs_avx512_vnni printed [SKIP] but the synthesis reported Passing: 1 with no skipped count, masking platform-dependent coverage as a successful execution.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real bug, fixed in 7991cae. The skip longjmp (rc=2) fell through to _exit(0) because oc_test_failed stayed 0. The child now exits 77 from the skip path; on this host the 4 AVX-512-gated tests show 'Skipped: 4' in the synthesis instead of counting as passes.
| fprintf(f, " <testsuite name=\"%s\">\n", t->suite); | ||
| } | ||
| fprintf(f, " </testsuite>\n</testsuites>\n"); |
There was a problem hiding this comment.
When more than one test is selected, this loop writes one opening <testsuite> per test but the function writes only one closing tag, so the XML artifact produced by the Capture test results step in .github/workflows/oxidize-c-ci.yml is malformed. Running the checked runner with --filter 'error/*' --xml ... selected five tests and an XML parser failed with mismatched tag, making the uploaded CI result unusable by JUnit consumers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real bug, fixed in 7991cae. write_xml now emits one balanced per suite with per-test rows (pass / / / ), fed by per-run records collected in the main loop. Verified the --xml output parses with a strict XML parser on the error/* filter.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Reviewed this as a follow-up: the shrink is real, the dead-code claims mostly hold, and the two Codex bugs (skip→pass, unbalanced XML tags) are actually fixed in 7991cae. A few things are still wrong or silently behavior-changing. Fixes are landing on Blocker — High
Medium
Not bugs (confirmed)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| printf("%s/%s%s\n", t->suite, t->case_name, | ||
| t->disabled ? " (disabled)" : ""); | ||
| return 0; | ||
| } else if (strcmp(argv[i], "--filter") == 0 && i + 1 < argc) { |
There was a problem hiding this comment.
Known flags without a value (--filter, --xml, --jobs, --verbose) miss the i + 1 < argc arm and fall through to g_filter = argv[i]. That matches nothing, prints Tested: 0, and exits 0 — a silent false green.
--pattern two branches later consumes its value and ignores it, so the Criterion alias is a no-op. --help is documented in CONTRIBUTING.md and hits the same unknown-as-filter path.
Reject missing values with usage + exit 2; wire --pattern to g_filter; implement --help.
There was a problem hiding this comment.
Fixed in 6613d42 — missing flag values now print usage and exit 2; --pattern is a real alias for --filter; --help/-h implemented; unknown long flags are rejected rather than silently becoming a no-match filter.
| } | ||
|
|
||
| int st = 0; | ||
| waitpid(pid, &st, 0); |
There was a problem hiding this comment.
waitpid is not EINTR-safe, and a -1 return leaves st == 0. WIFEXITED(0) && WEXITSTATUS(0) then reports PASS for a test whose child we never successfully waited on (possible zombie).
Loop on EINTR; any other wait failure should be RESULT_CRASH.
There was a problem hiding this comment.
waitpid is now EINTR-safe and a wait failure returns RESULT_CRASH instead of falling into WIFEXITED(0) — fixed in 6613d42.
| s->tested, s->failing, s->crashing); | ||
| /* One <testsuite> per suite, one <testcase> per test — balanced tags | ||
| * for JUnit consumers (CI uploads this artifact). */ | ||
| for (OcTest *st = oc_tests_head; st; st = st->next) { |
There was a problem hiding this comment.
This still duplicates every suite once per registry entry (including disabled tests). st walks oc_tests_head, and every hit with the same st->suite re-opens the suite and re-emits every matching runs[] row.
A suite with 20 cases therefore appears 20 times in the JUnit file the Capture test results step uploads. Tags are balanced now, but the artifact is ~N² the real test count.
Iterate unique suite names (or walk runs and open/close on name change), and actually use suite_tests / suite_fails on the <testsuite> tag instead of discarding them.
There was a problem hiding this comment.
Fixed in 6613d42 — write_xml walks the run records once, opens each suite on name change, and emits tests/failures/errors attributes per suite. Verified: 184 unique suites, 2929 cases, parses clean.
| break; | ||
| case RESULT_SKIP: | ||
| stats.skipped++; | ||
| stats.passing++; /* criterion counts skips as non-failures */ |
There was a problem hiding this comment.
Skip is not a pass. stats.passing++ here makes Passing: N include skips, so AVX-512-gated tests inflate the pass count on AVX2 hosts. Criterion treated skip as a non-failure, not as a pass — keep it only in Skipped.
Same switch: RESULT_CRASH also increments failing, and write_xml then publishes failures=failing and errors=crashing, so crashes are counted twice in the JUnit summary.
There was a problem hiding this comment.
Fixed in 6613d42 — skips no longer increment Passing (2925 pass + 4 skips on AVX2 hosts now) and RESULT_CRASH no longer double-counts into failing; the JUnit summary uses failing for failures= and crashing for errors=.
| // Bound the ring by the same cap the spec streams use: a stop | ||
| // sequence longer than this can never match a realistic window and | ||
| // an untrusted config must not drive unbounded pre-allocation. | ||
| const MAX_STOP_SEQUENCE_LEN: usize = 4096; |
There was a problem hiding this comment.
The CodeQL clamp is fine as an allocation bound, but matching still runs ends_with against sequences longer than the ring. A stop sequence of length 4097 can never match after this, and the PR described it as no behavior change.
Keep the 4096 cap at the allocation site, but only consider sequences with len <= max_len so the invariant is explicit (and add a test). Pathological configs stay bounded; realistic ones are unchanged.
There was a problem hiding this comment.
Fixed in 6613d42 — the matcher now filters len <= max_len explicitly, with two unit tests covering both a matching sequence and a 5000-token over-long one next to a normal one.
|
|
||
| let mut row_major = vec![0.0_f32; rows.saturating_mul(batch)]; | ||
| let compute_row = $compute_row; | ||
| row_major |
There was a problem hiding this comment.
The no-AVX2 arm of this macro always par_chunks_muts. On master, gemm_iq1_s_decode_once / iq1_m / nvfp4 used total_ops >= PARALLEL_GEMV_MIN_OPS with a serial else (q4_k / q8_0 were already always-parallel).
Outputs should match, but small batches now pay rayon overhead they didn’t before. Restore:
let total_ops = rows.saturating_mul(cols).saturating_mul(batch);
if total_ops >= PARALLEL_GEMV_MIN_OPS { par... } else { serial... }in both macro arms.
There was a problem hiding this comment.
Fixed in 6613d42 — both macro arms now gate on rowscolsbatch >= PARALLEL_GEMV_MIN_OPS with a serial else, restoring the pre-macro scheduling for the iq1_s/iq1_m/nvfp4 wrappers.
| * `hd` floats each, in place. The arithmetic is identical at both call | ||
| * sites (prefill parity is a hard invariant). */ | ||
| static void llama_rope_dispatch(float *vecs, uint32_t n, size_t hd, | ||
| size_t rope_dim, size_t pos, float rope_theta, |
There was a problem hiding this comment.
oc_apply_rope_f32 / _norm / _yarn take int64_t position, and both call sites pass s->pos / pos0 + (int64_t)j as int64_t. Widening through size_t wraps any negative position and truncates on ILP32.
This helper exists specifically to keep forward and prefill bit-identical — keep the parameter as int64_t pos.
There was a problem hiding this comment.
Fixed in 6613d42 — llama_rope_dispatch now takes int64_t pos, matching the oc_apply_rope_* signatures and both call sites.
| #include "oxidize/gguf.h" | ||
| #include "oxidize/llama.h" | ||
|
|
||
| #include "llama_session_ops.h" |
There was a problem hiding this comment.
After deleting the GLM/Hunyuan forwards this file no longer calls anything in llama_session_ops.h, matvec.h, quant.h, tensor_ops.h, activation.h, arena.h, or log.h. The “keep local copies of llama.c helpers” comment is also leftover from the deleted machinery.
The public header still describes “architecture-specific forward functions” that this PR removed. Worth tightening so the next reader doesn’t go looking for oc_arch_forward_glm.
There was a problem hiding this comment.
Fixed in 6613d42 — the stale includes (llama_session_ops.h, matvec.h, quant.h, tensor_ops.h, activation.h, arena.h, log.h, model.h) are removed, and the public header intro no longer describes the deleted forward functions.
|
Final state after review feedback:
Not merging this — leaving it ready for human review. |
Clang 18 does not define __SANITIZE_ADDRESS__, so also gate child signal handlers on __has_feature(address_sanitizer). Parse --jobs and --verbose with strtol, print comparison operands before caller format args, and name QK-norm on OcLlamaLayer / OcGlmConfig instead of OcLlamaConfig. Co-authored-by: dogesman098 <dogesman098@gmail.com>
…nd oxk - inf_model: drop per-element swiglu/geglu pass that the full-vector pass immediately overwrites, and the byte-identical duplicate attention block in oc_inf_model_forward_batch that ran the whole attention pass twice - oxk_avx512: drop dead first loop in oc_oxk_dot_q8_0_q8_0_avx512_vnni whose accumulator was discarded before the second loop recomputed it - glm_arch/arch_forward: drop never-compiled OC_*_TEST_STUBS sections No behavior change; batch forward now does one attention pass instead of two. Full suite: 2923/2923 passing.
Replace 16 hand-rolled per-type dequant/pack functions with two DEFINE_PLAIN_DEQUANT / DEFINE_PLAIN_PACK macro families plus shared LE load/store helpers (f32/u64/bf16). Same validation, same loop bodies, same byte layout; test_quant VAL-QUANT-008/016 bit-exactness suites still pass (44/44), full suite 2923/2923.
Remove the 2.6 MB prebuilt libcriterion.a and 9k lines of vendored headers in favor of tests/framework.h + tests/framework_main.c (~600 lines total), implementing exactly the API surface the suite uses: Test() auto-registration, cr_assert*/cr_expect* families with optional messages, .description/.disabled extras, cr_skip_test, fork-per-test isolation, --filter/--list/--xml/--jobs. - Kills the glibc >= 2.38 pin that forced ubuntu-24.04 in CI - macOS no longer needs brew criterion; the runner links only libc/libm/libpthread - Drops 5 dead <criterion/redirect.h> includes - Updates Makefile, CONTRIBUTING.md, and both oxidize-c workflows All 2927 tests pass locally under the new runner (no sanitizer); the full ASan+UBSan matrix runs in CI.
308 trivial Test() bodies that only call fn(NULL) / assert OC_ERR_INVALID_ARG become OC_TEST_NULL_SAFE / OC_TEST_REJECTS_NULL one-liners in framework.h. The exercised expressions stay visible at the call site; test count and coverage unchanged. 2927/2927 passing.
- New src/format/tokenizer_common.h (private): OcU64Map open-addressing map + oc_pair_key + oc_tokenizer_string_array - tokenizer_bpe.c / tokenizer_tiktoken.c drop byte-identical duplicate u64map implementations (~250 lines) - bpe/sp/wp drop three copies of the GGUF string-array loader (~66 lines) - Bit-exact VAL-TOK tokenizer suites pass (46/46)
Master's sampling.rs top-k heap commit landed unformatted; CI's newer rustfmt formats the let-chain and collect differently than the committed text. Apply canonical formatting so 'Check formatting' passes again.
qwen/mistral/gemma/phi reference engines share byte-identical blocks: RMSNorm (12 sites), row-major matvec (28 sites), per-head RoPE (8 sites), tanh-GELU (2 local defs). Extract to static-inline helpers in src/model/arch_ops.h. Public API and forward-pass semantics unchanged; per-arch tests pass (62/62).
embed_token / matvec / attention_head existed as byte-identical static copies in llama.c, arch_forward.c, and glm_arch.c (the latter two even documented the duplication). Extract into private src/model/ llama_session_ops.h; llama.c keeps only its gemma4-scale + muse embedding post-processing wrapper. Full suite 2929/2929, including the prefill-parity invariants.
Delete pub functions with zero callers across the whole workspace: - kernels/transposed.rs: the entire quantized-transposed gemv family (qk/q4_k/q6_k/q8_0, gemv_quantized_f32_transposed dispatcher, q4 AVX2/AVX-512 accumulate helpers) — only gemv_f32_transposed is live (dflash, cpu_kernels, CUDA dispatch) - activation.rs: rms_norm_gemv_f32_transposed, layer_norm_f32, scaled_dot_product_attention_f32 (test-only) - gemm.rs: gemm_i8, gemm_i4 (+cpu variants, unpack_i4), linear_activation_f32 (test-only) - q_kernels.rs: gemv_qk_f32_fused (0 callers) - gemm_decode.rs: dot8_f32_avx2 (was #[allow(dead_code)]) - cpu_kernels.rs: CpuWorkspace, fused_rms_norm_gemv_f32_transposed, matmul_reuse_workspace, dot_product_avx512_or_scalar (self-referenced only); keep dot_product_avx2_or_scalar (activation_stats) and the public error/kernel-registry types Their unit tests are removed with them; remaining coverage: oxidize-core 642 passed. clippy -D warnings clean (one pre-existing master warning).
GgufMetadataValue::as_u32/as_f32 now live next to the enum; the byte-identical 10-arm lookup matches in inference.rs (u32 + array-max + f32), format/tokenizer.rs, and mlx_inference.rs collapse to one-liners. dflash/eagle3/fingerprint keep their local coercions (different accepted type sets). Tests: oxidize-core 642 passed.
oc_gemv_dispatch! stamps the shared wrapper skeleton (3-way shape validation, PARALLEL_GEMV_MIN_OPS row-parallel/serial dispatch) that was hand-copied in every per-quant wrapper; the per-row closure (AVX2 dispatch + scalar fallback) stays verbatim at each call site. Converted q4_k / q6_k / iq4_xs (+ iq4_nl_q8 kept hand-written: its wrapper owns the activation quantization). Kernels themselves untouched. oxidize-core 642 tests pass, clippy clean.
oc_gemm_decode_dispatch! stamps the 6 gemm_*_decode_once wrappers (validation + AVX2 fast-path hook + row-major panel compute + transpose epilogue); per-quant row-compute closures stay verbatim at each site. Kernel math untouched. 642 tests pass, clippy clean.
- Doc comments on macro invocations (rustdoc can't attach them) become regular comments at the q_kernels/gemm_decode dispatch sites - generation.rs: the constant assert that broke clippy -D warnings on master (all 3 OSes) becomes a const-evaluated check - Full workspace clippy is now clean, unblocking the failing 'CI' workflow that master has been failing since PR #42
setup_tiny_model was copy-pasted in 4 test files (inf_forward, gen_loop, layer_range, layer_wise) with only context_size differing (32 vs 64). Now one oc_test_setup_tiny_model(model, ctx) in tests/tiny_model.h. Same deterministic weights, same tests, same coverage. 2929/2929 pass.
The four generation streams (Speculative/Mtp/Eagle3/plain) each carried byte-identical emit_token stop-sequence logic (4 copies) and update_speculation_health bookkeeping (3 copies). Extract StopTracker (bounded recent-token ring + stop matching) and SpeculationHealth (drafted/accepted totals, zero-accept streak, disable rule); streams delegate. One behavior-neutral clone is added in GenerationStream's poll_next to satisfy the borrow checker. 642 tests pass, clippy clean.
9 identical OnceLock env-flag functions (OX_GPU_LAYER_Q8K, LMHEAD_Q8K, GEMV_MW, FUSED_MMQ, FUSED_QKV, FUSED_MW, FFN_FUSE, BATCHED_DECODE) each collapse to a one-line macro invocation in backends/cuda.rs. Same read-once semantics, same polarity rules. clippy clean, 642 tests pass.
llama_rope_dispatch (3-way YaRN/norm-pairs/plain, 4 duplicated copies across forward and prefill) and llama_qk_norm_heads (QK-norm loops, 2 copies) become single static helpers used by both paths. Prefill-parity tests pass (muse_glimmer, qwen35_forward, longcat); 2929/2929 total.
oc_arch_forward_glm / oc_arch_forward_hunyuan and their 14 static helpers (~800 lines) were never reachable: no loader populated their sessions, llama.c's dispatch never selected them, and no test executed them (test_glm_arch covers config parsing, version strings, and arch enums only). GLM/Hunyuan inference runs through the llama.c session paths. Config parsing, defaults, version mapping, and arch-registry entries are kept and tested (21/21 glm_arch tests pass).
The AVX2 fast path is cfg'd to x86; on aarch64 (macOS CI) the closure param was unused. Reference it unconditionally on the scalar path.
The EMIT / EMIT_U8/U32/U64/F32 / EMIT_KV_STR_KEY family was redefined (and #undef'd) 21 times across 5 test files. One tests/gguf_emitter.h now defines them; identical host-order emit behavior. 46 tokenizer + 27 gguf + 29 writer tests pass; full suite 2929/2929.
The three rust/uncontrolled-allocation-size alerts master carries from PR #42 (StopTracker ring, draft/emit buffer capacities, partial top-k heap) each already had runtime filters, but the bounds were not visible to static analysis at the allocation site. Add inline min() clamps and a hard cap on the stop-sequence ring (4096); no behavior change for any realistic config.
Review findings on the new framework: - cr_skip_test longjmp'd with rc=2 but the child still exited 0, so skips were recorded as passes (platform-conditional tests silently counted as executed). Exit 77 from the skip path; synthesis now shows 'Skipped: 4' for the AVX-512-gated tests on non-AVX-512 hosts. - write_xml opened one <testsuite> per test but closed only one, producing malformed JUnit XML. Rewrite with per-suite grouping and per-test <testcase> rows (pass/fail/crash/skip); xmllint-clean.
- framework_main.c: EINTR-safe waitpid (wait failure is a crash, never a pass); skip no longer counts as Passing and crash no longer double- counts in failing; --xml walks runs once (no per-test suite duplication, real tests/failures/errors attrs on each suite); flag values are required (--filter/--pattern/--xml/--jobs/--verbose reject missing values with usage, exit 2), --pattern is a real filter alias, --help/-h implemented, unknown long flags rejected instead of silently matching nothing - llama.c: llama_rope_dispatch takes int64_t pos, matching the oc_apply_rope_* signatures and both call sites (no size_t wrap) - generation.rs: StopTracker only matches sequences that fit the ring (explicit len <= max_len filter) + 2 unit tests - kernels.rs: oc_gemm_decode_dispatch restores the pre-macro PARALLEL_GEMV_MIN_OPS gate with a serial else in both arms so small batches skip rayon dispatch - glm_arch: drop includes left from the removed forward machinery; header no longer describes deleted forward functions oxidize-core 644 tests pass; C suite 2925 pass + 4 skips; clippy clean.
7991cae to
6613d42
Compare
Jackson57279
changed the title
|
Rebased onto master (after #49's overlapping artifact removal — my hygiene commit was absorbed), and addressed the second review round in 6613d42 + fe9e827:
All 4 workflows green on fe9e827; CodeQL and cubic neutral with 0 open alerts. Still not merging — ready for human review. |
2 participants
Summary
I removed 15,649 lines across the workspace with no features lost, and every CI workflow on this branch is green — including two that master was failing. The tree is smaller, several hot paths got faster, and the C test suite now runs on a ~600-line in-repo framework instead of a 2.6 MB vendored binary.
(Note: master also merged artifact removal in #49 after this branch did its own; the net source-code delta here is ~12.5k lines on top of that.)
oxidize-core: 644 tests pass (2 new); workspaceclippy -D warningsclean;cargo fmtcleanoxidize-c: 2,925 tests pass + 4 hardware-conditional skips now correctly reported as skips; full ASan+UBSan matrix (gcc + clang) green in CIuncontrolled-allocation-sizealerts; fixed here)What I did
Test infrastructure
libcriterion.a) withtests/framework.h+tests/framework_main.c(~640 lines):Test()auto-registration, thecr_assert*/cr_expect*families with optional messages,.description/.disabled,cr_skip_test, fork-per-test isolation,--filter/--pattern/--list/--xml/--help. Un-pins CI from glibc >= 2.38 and drops the macOSbrew install criterionstep.EMIT_*macros (21 redefinitions).--help.Dead code removal (verified zero callers across all 12 crates + tests)
oxidize-c: the never-reachable GLM/Hunyuan forward machinery (~750 lines); a byte-identical attention block inoc_inf_model_forward_batchthat ran the whole attention pass twice; a dead per-element swiglu pre-pass; a dead first loop in an AVX-512 VNNI kernel; never-compiled stub sections.oxidize-core: the quantized-transposed GEMV family, test-onlygemm_i8/i4/layer_norm/sdpa/linear_activation,gemv_qk_f32_fused,dot8_f32_avx2,CpuWorkspacehelpers (~1,524 lines incl. orphaned tests).Consolidation of real duplication
oxidize-c/src: macro-templated plain-type dequant/pack (VAL-QUANT bit-exact suites still pass); shared tokenizeru64map/string-array helpers; de-triplicated llama session ops;arch_ops.hfor the 4 reference engines; sharedllama_rope_dispatch+llama_qk_norm_headsfor forward/prefill (parity tests green;int64_tposition preserved per review).oxidize-core:oc_gemv_dispatch!/oc_gemm_decode_dispatch!stamp wrapper skeletons (kernels verbatim; parallel threshold restored per review);GgufMetadataValue::as_u32/as_f32;StopTracker+SpeculationHealthreplace 4+3 copies (ring/match invariant now explicit + tested);ox_env_flag!collapses 9 env-flag getters.Fixes master inherited
cargo fmtfailure insampling.rs; clippy-D warningsfailure ingeneration.rs; the 3 CodeQL allocation alerts (bounds now statically visible at each allocation site).Speed wins
oc_inf_model_forward_batch: one attention pass per layer instead of twoInvariants untouched
Review trail
Two AI-review rounds, 14 findings, all resolved: framework bugs (skip accounting, XML balance/duplication, EINTR waitpid, flag handling), the
size_t->int64_trope position narrowing, the macro parallelism threshold regression, the StopTracker ring/match invariant (with tests), stale glm_arch includes/docs, the CodeQL bounds, and the removed-API concern (answered in-thread: oxidize-core is unpublished — crates.io 404 — so no downstream importers exist).