Production hardening round 4: max_tokens bug, overflow hardening, honesty pass, CI - #1
Merged
Merged
Conversation
added 10 commits
July 12, 2026 09:05
Establishes a formatting-clean baseline so 'cargo fmt --all -- --check' passes (it previously failed with many diffs). No behavior change.
length_budget_policy(max_tokens) and the length check inside
combined_policy(...) compared the approximate output token count
(GET_TOKEN_COUNT) against the enforcer's conservation budget
(GET_BUDGET, syscall 10) instead of the configured max_tokens
parameter. The parameter was completely unused (compiler warning),
so the 'length budget' threshold always tracked the decay budget.
The bug was masked because every existing test and README example
passed identical values for max_tokens and the enforcer budget.
Now both policies load max_tokens into R3 directly
(MOVI R3, {max_tokens}) and compare the token count against it.
Added two regression tests that decouple max_tokens from the budget:
- length_threshold_is_max_tokens_not_budget: tiny max_tokens + huge
budget still blocks an oversized output.
- length_allows_under_max_tokens_even_when_budget_low: output above
the budget but under max_tokens is allowed.
Both fail against the previous implementation.
GET_REPETITION, GET_CATEGORY, and GET_UNIQUE_RATIO each compute a per-mille ratio as (count * 1000) / total using u32 arithmetic. When a single word appears more than ~4.29M times (count * 1000 > u32::MAX) the multiply overflows: a debug build panics and a release build silently wraps, producing a wrong (often tiny) ratio and thus a wrong enforcement decision. All three now accumulate counts in u64 before multiplying by 1000, casting back to u32 only for the register store (the ratio is bounded to 0..=1000 so it always fits). GET_REPETITION is also rewritten to count each distinct word once (iterator-based) rather than re-scanning the whole output for every token, which additionally makes the large-input case O(total) instead of O(total^2). This also clears the clippy needless_range_loop lint. Added syscall_ratios_no_overflow_on_large_input: feeds 4.5M identical tokens through GET_REPETITION and GET_UNIQUE_RATIO. Verified it panics with 'attempt to multiply with overflow' under the old u32 math.
cargo build/clippy were emitting warnings that would fail a strict CI gate (clippy --all-targets -D warnings previously errored). Fixed: - RegisterFile::set: drop the no-op 'val & 0xFFFFFFFF' mask and the if/else with identical arms; 'val as i32' already reinterprets the bits for the sign flag. - parse_label: use the line_num argument (it was unused) in the duplicate-label error message, and simplify the call site to '?'. - simple_hash: gate behind #[cfg(feature = "audit")] since it is only referenced from audit-gated code (was dead-code without the feature). - scope_discipline_policy: drop a useless format!() and the dummy 'let _ = i' by using a '_' loop binding. - is_leap (audit feature): use is_multiple_of instead of manual '%'. - tests/integration.rs: remove unused EnforcementResult import. No behavior change.
The density_boundary test claimed to check '1 unique / 2 total = 500 per-mille, exactly at threshold' but used the output 'hello world', which has 2 unique words out of 2 (ratio 1000) - well above the 500 threshold. The test therefore passed regardless of where the real boundary sat and could not detect an off-by-one (e.g. JLE vs JLT). Now uses 'go go' (1 unique / 2 total = 500, equal to the threshold, allowed because JLT is strict) and 'go go go' (1/3 = 333, just below the threshold, must be blocked) so both sides of the boundary are pinned. Updated in both the lib unit tests and integration tests.
AuditLog::read_all was declared to return Vec<serde_lite::JsonValue> where JsonValue was an empty (uninhabited) enum in a private module. That made the method silently always return an empty vector - a stub dressed up as a working API. It now returns Vec<String> of the non-empty JSONL records actually on disk (empty if the file is missing/unreadable), which is genuinely useful since the crate intentionally has no JSON dependency. The bogus serde_lite placeholder module is removed. Also silence the feature-gated clippy::too_many_arguments on AuditLog::log (it takes a flat audit-record argument list) so the audit feature passes 'clippy --all-features -D warnings'. Added a feature-gated test that logs two records and asserts read_all returns both with the expected fields (and agrees with summary()).
Verified each public claim against the actual code and fixed the mismatches rather than overselling: - no_std: 'cargo build --no-default-features' fails (~90 errors; the code uses String/Vec/format!/HashMap). The README claimed 'No_std compatible - Works in embedded, WASM, and kernel contexts'. Replaced with an honest status marker: no_std/embedded is a stated goal, not a working configuration. The crate does build for wasm32-unknown-unknown with std, so that is now the advertised target. - Cargo.toml categories: dropped the false 'no-std' and 'embedded' crates.io categories (kept 'wasm', which is verified). - Test count: integration.rs has ~80 tests (not '95+'); total is 150+. Updated the badge and the Architecture section accordingly. - Cross-implementation claims: removed the unverified assertions that this is a 'line-by-line port of the Python v0.2.0', that 'the Python 95-test suite has been replicated', and that bytecode is 'binary-compatible' across implementations. None of these are checked by CI. Reworded as independent implementations of the same ISA with cross-compatibility as an unverified goal.
The previous workflow ran 'cargo clippy -- -D warnings' (no --all-targets), so warnings in tests/integration.rs (e.g. the unused EnforcementResult import) were invisible to CI, and there was no formatting gate at all. That is how the formatting drift and the test-file unused import shipped. Strengthened the pipeline: - 'cargo fmt --all -- --check' step. - clippy now runs with '--all-targets --all-features -- -D warnings' so lib, unit tests, integration tests, and the audit/metrics features are all covered. - Tests run under both default features and --all-features (exercises the audit read_all test). - Added a dedicated wasm32-unknown-unknown build job to back up the README's WASM-ready claim. - fail-fast: false so a beta-only failure still surfaces stable results.
…6-07-11 # Conflicts: # .github/workflows/ci.yml # README.md
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.
Summary
length_budget_policyandcombined_policysilently ignoredmax_tokensentirely — the generated bytecode compared output length against the enforcer's decay budget instead of the configured parameter (compiler-confirmed unused variable). Masked because every existing test/README example passed identical values for both. Fixed; added two regression tests that decouplemax_tokensfrom the budget (tiny max_tokens + huge budget still blocks; large max_tokens + tiny budget still allows), mutation-verified against the old buggy code.GET_REPETITION/GET_CATEGORY/GET_UNIQUE_RATIO: per-mille ratio math multiplied counts by 1000 inu32before dividing, silently wrapping to a wrong (often tiny) ratio on large inputs. Now accumulates inu64first.GET_REPETITIONalso rewritten from an O(n²) re-scan to an O(n) iterator pass. New regression test feeds 4.5M identical tokens through both syscalls, confirmed to panic with "attempt to multiply with overflow" under the oldu32math.AuditLog::read_allwas a stub dressed up as a working API — typed to returnVec<serde_lite::JsonValue>whereJsonValuewas an uninhabited enum in a private module, so it always silently returned an empty vector. Now genuinely reads and returns the raw JSONL records.test_density_boundary) claimed to test "1 unique/2 total = 500 per-mille, exactly at threshold" but actually used"hello world"(2 unique/2 total = 1000 per-mille) — nowhere near the boundary, so it couldn't detect an off-by-one. Fixed to use inputs that actually land on both sides of the threshold.no_stdwas advertised as supported but doesn't actually compile with--no-default-features(usesString/Vec/HashMapunconditionally) — downgraded to "stated goal, not working," with the realwasm32-unknown-unknown(withstd) support advertised instead (build-verified). Corrected a false "95+ tests" claim (~80 integration, 150+ total). Removed unverified cross-implementation claims ("line-by-line port," "binary-compatible bytecode") that nothing in CI actually checks.cargo fmt --all -- --check, switched clippy to--all-targets --all-features -- -D warnings(the old invocation missed warnings in the test files — exactly how the formatting drift and an unused import shipped unnoticed), tests under both default and all-features, and a dedicated wasm build job backing the README's WASM claim.Test plan
cargo test --all-features— 156/156 passing (was 152 baseline; fresh clone, independently re-verified)cargo clippy --all-targets --all-features -- -D warnings— cleancargo fmt --all -- --check— cleancargo build --target wasm32-unknown-unknown— succeeds🤖 Generated with a non-Claude coding agent (opencode/GLM), independently verified via fresh clone before opening this PR.