Skip to content

Correctness and stability fixes from the maturity audit (findings #1, #2, #3, #6) - #12

Merged
paulprogrammer merged 7 commits into
mainfrom
maturity-work
Jul 17, 2026
Merged

Correctness and stability fixes from the maturity audit (findings #1, #2, #3, #6)#12
paulprogrammer merged 7 commits into
mainfrom
maturity-work

Conversation

@paulprogrammer

@paulprogrammer paulprogrammer commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

First tranche of fixes from the codebase maturity audit (maturity_report.md, included in this PR with per-finding status entries). Covers the report's recommended items 1 and 2: the pretty-printer round-trip fixes and the runtime safety trio, plus a CI-flakiness fix that the work surfaced.

Finding #1 — PrettyExpr emitted source the parser cannot re-read (fec93f0)

The MCP patch_symbol tool rewrites whole files through PrettyExpr, so every printed construct that couldn't re-parse silently corrupted patched files. Fixed all 8 audited bugs (DeBruijn D0→^0, ju arg order, swapped !/`, Eq/Gt glyphs, export prefix, expand params) plus 4 more found while writing tests (bitwise ops printed ???, Import emitted a trailing arity, whole floats re-lexed as integers, string escaping missed \/\n/\t/\r). Adds an 11-test parse→print→parse round-trip suite; corrects the inverted doc-comments in ast/mod.rs that caused the bug.

Finding #6 — MCP index corruption and wrong-file patching (7afff91)

Re-analysis appended to the fingerprint index without clearing (duplicates on every patch; deleted symbols persisted), the trailing-newline write could abort the server via bare .unwrap(), and patch_symbol("main") silently rewrote whichever file last won the name-keyed index. The index is now per-file and purged on re-analysis (staleness impossible by design), ambiguous names are rejected with the candidate list, and a new optional path argument disambiguates. run_semantic_patch.py asserts both behaviors and now runs in CI on both platforms.

Finding #2 — llm_read raw-fd path: leak, data loss, null-deref (88394b1)

A fresh fdopen per read leaked the FILE* and its read-ahead swallowed every line after the first; NULL return crashed. Now a byte-at-a-time read() loop: nothing leaks, consecutive reads return consecutive lines, bad fds return 0. Regression test reads three piped lines + EOF; verified to fail against the old code.

Finding #3 — non-atomic refcounts (487633f, c697784)

ref_cnt was a plain unsigned short mutated with ++/-- while llm_fork shares managed pointers across pool threads: lost updates → premature free → use-after-free. Now _Atomic unsigned int (fetch_add/fetch_sub, acq_rel on release), with LlmRtHeader reordered to three padding-free u32 fields and gen_string_constant updated in lockstep. New 8-thread × 200k dup/drop race test (fails against old code, passes now, both platforms). Also: llm-clang now rebuilds runtime objects when common.h changes instead of silently linking stale layouts.

Test flakiness — httpbin.org dependence removed (386de70)

http_live_test.llm and https_json_test.llm called httpbin.org, whose outage turned CI red. The llm-test mock server now provides httpbin-style JSON echo endpoints (/json/get, /json/post) and both tests target it; verified green during the outage.

Testing

The maturity-work branch stays open — remaining findings continue there.

🤖 Generated with Claude Code

paulprogrammer and others added 7 commits July 17, 2026 07:20
@paulprogrammer @claude
docs: add maturity report — codebase scan of inefficiencies, redundan…
dccf502 
…cies, and antipatterns

39 findings across four tiers (correctness/safety, performance,
redundancy, hygiene) with impact and difficulty estimates, pinned
to v0.5.0 / main @ 3ebb79d. This branch tracks the maturity work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: make PrettyExpr output re-parseable (12 round-trip bugs)
fec93f0 
The MCP patch_symbol tool rewrites whole source files through
PrettyExpr, so any printed construct the parser cannot re-read
silently corrupted patched files.

Fixed (verified against the lexer/parser):
- DeBruijn printed `D0` instead of `^0` (hit every bound variable)
- ju arg order was flipped (shape before expr)
- Panic/Expand glyphs were swapped (`!` vs `\``)
- Eq printed `==`, Gt printed `>` (which lexes as Move)
- export prefix printed `*` (Mul) instead of `X `
- expand params printed `!name` instead of `name\``
- format_token emitted `???` for BitAnd/BitOr/BitXor
- Import emitted a trailing arity the parser rejects
- whole floats printed without `.0` and re-lexed as Integer
- string escaping missed backslash/newline/tab/CR

Also corrects the inverted `!`/`\`` doc-comments in ast/mod.rs that
display.rs had been written against, and adds an 11-test
parse->print->parse round-trip suite covering every construct plus
real files from tests/lang/.

Resolves maturity report finding #1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: MCP index integrity and unambiguous patch_symbol targeting
7afff91 
The codebase index kept three name-keyed maps that analyze_path only
ever appended to: every re-analysis (including the one patch_symbol
triggers after each write) duplicated fingerprint entries, and
renamed or deleted symbols persisted forever. Names were also global,
so patch_symbol("main") silently rewrote whichever file last won the
index — observed corrupting otel_async_test.llm during the display.rs
validation.

- Restructure the index: call_graph/fingerprints side maps folded
  into per-symbol metadata, keyed name -> one entry per defining
  file; re-indexing a file purges its old entries first, so stale or
  duplicate data is impossible by design
- patch_symbol now rejects ambiguous names with the candidate file
  list and accepts an optional 'path' argument to disambiguate
- Replace the unwrap-based trailing-newline append (a server-killing
  panic under panic=abort if the file races away) with a single write
- run_semantic_patch.py asserts the ambiguity rejection and the
  disambiguated patch, and now runs in CI on both platforms

Resolves maturity report finding #6.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: llm_read raw-fd path leaked FILE* and lost data past the first line
88394b1 
fgets over a fresh fdopen per call had three defects: the FILE* was
never fclosed (one leak per read), its readahead buffered up to 4096
bytes and dropped everything after the first line when the stream was
abandoned (subsequent reads saw EOF), and a NULL fdopen return went
straight into fgets (crash on a bad fd).

Replace the raw-fd path with a byte-at-a-time read() loop (EINTR
retried): nothing to leak, nothing consumed past the newline —
consecutive reads return consecutive lines — and a bad fd returns 0.
The managed LlmFile path already cached its FILE* and is unchanged.

Adds tests/lang/stdin_multiline_test.llm (three piped lines + EOF),
run by llm-test with piped stdin; it fails with 'line 2 mismatch'
against the old code and passes now.

Resolves maturity report finding #2.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: remove live httpbin.org dependence from the test suite
386de70 
http_live_test.llm and https_json_test.llm called httpbin.org, so any
outage of that service turned CI red (observed 2026-07-17: both tests
failing with 503s on every platform while the rest of the suite was
green).

The llm-test mock server now emulates the two httpbin behaviors the
tests need — /json/get returns a JSON body echoing the request URL,
/json/post echoes a form-encoded body back as a JSON 'form' object —
on paths distinct from the plain-text /get and /post that
http_test.llm asserts. Both tests now target the mock; the full suite
passes with httpbin.org unreachable (verified during the outage).

TLS client coverage is unchanged: it lives in https_test.llm /
run_https_test.sh, not these tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: atomic 32-bit refcounts on managed objects
487633f 
ref_cnt was a plain unsigned short mutated with ++/--, while llm_fork
hands the same managed pointers to pool threads: racing dup/drop lost
updates, freeing objects that still had references (use-after-free),
and the 16-bit counter could wrap.

- ref_cnt is now _Atomic unsigned int: dup uses fetch_add (relaxed),
  drop uses fetch_sub (acq_rel) and only the releaser of the last
  reference destroys the object
- LlmRtHeader reordered to {magic: u32, type: u32, ref_cnt: u32} —
  three 4-byte fields, no padding — and gen_string_constant, which
  materializes this header for string constants, updated in lockstep
  ({i32,i32,i32} instead of {i32,i16,i16})
- new tests/runtime/refcount_race_test.c (8 threads x 200k balanced
  dup/drop, wired into llm-test): fails against the old code with
  'object was freed while references remained', passes now
- llm-clang now rebuilds runtime objects when common.h is newer than
  the object, so header layout changes cannot silently link stale
  objects built against the old struct

Resolves maturity report finding #3.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer @claude
fix: link refcount race test on macOS
c697784 
Mach-O ld does not resolve undefined weak references to NULL the way
GNU ld does, so the standalone test link failed on the runtime's weak
drop hooks. Provide no-op stubs — the test only allocates strings, so
they are never invoked.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@paulprogrammer
paulprogrammer merged commit 0aa680b into main Jul 17, 2026
4 checks passed
paulprogrammer added a commit that referenced this pull request Jul 17, 2026
@paulprogrammer @claude
perf/hygiene: cheap-wins batch (findings #12, #13, #18, #22, #36)
f2d4284 
- #12: trap/parallel IDs from a monotonic counter instead of
  get_functions().count() per site (O(n^2) codegen on trap-heavy code)
- #13: module name and mangle prefix computed once in CodeGen::new
  instead of re-parsing/re-hashing on every identifier resolution
- #18: pthread_once curl_global_init shared by http.c/http_server.c,
  removing the lazy per-easy init race across pool threads
- #22: rust-cache + concurrency groups in ci.yml and release.yml
- #36: set -eo pipefail in llm-clang/llm-test; runtime compile and ar
  failures abort with a message instead of linking stale objects;
  brew --prefix instead of hardcoded Homebrew paths; libllm_opencl.so
  copied only when stale; llm-test fails fast with diagnostics when
  the mock server cannot bind port 8080

Validated: cargo test (47 passed), llm-test (25 passed, fresh runtime
objects), run_semantic_patch.py, and a forced port-8080 conflict now
exits 1 with the bind error surfaced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@paulprogrammer