Skip to content

module cache: sources through FileAccess, content stamps, a complete policy guard; dasLLAMA tests get a per-PR split and areas - #3938

Merged
borisbat merged 19 commits into
masterfrom
bbatkin/module-cache-followup
Sep 5, 2026
Merged

module cache: sources through FileAccess, content stamps, a complete policy guard; dasLLAMA tests get a per-PR split and areas#3938
borisbat merged 19 commits into
masterfrom
bbatkin/module-cache-followup

Conversation

@borisbat

@borisbat borisbat commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Rebuild required for anything compiled against include/daScript/ast/ast_serializer.h: AstSerializer gained two members and the module cache's record header changed (stream version 204), so every existing .dascache rewrites itself once.

The module cache served a module's file by name and length and then read the bytes back from disk. That was the wrong layer: a compile's sources come from its FileAccess, which may be a .das_project hook or an in-memory file system. A served module's FileInfo now resolves through the compile's access, and the module takes its own file from the access the way a parse does. The record stamps the source by content (size plus hash of the bytes the access serves) instead of stat mtime and size, so a file that is not on disk under that name is covered, a byte-identical rewrite still serves, and a same-size edit is a cutoff. A record also carries every CodeOfPolicies field of the compile that wrote it; a record whose policies differ is never served. The compare derives version_2_syntax from the source the way the parse does, so a -v1syntax host serves a gen2 module instead of rewriting the cache every run. A policy mismatch fails like a damaged record: reparsed in place, repaired by the writeback, never a cutoff.

The dasLLAMA test runner splits its per-PR gate in two. model-free holds the files that reach no model or fixture and runs the same on a bare box; stocked holds the model-gated files and is the per-PR coverage on a box with models. Twelve test files sat in no suite and are now filed; one of them, test_chat, had been red since gemma-4 flipped to thinking by default, and its instruct-wire cells now opt out. --exclude drops files loudly. --area audio|vision|tts|llm|infra runs what someone working on that area needs, and --changed derives the areas from the diff. Every run ends with REPORT and NEXT. A census test holds the suite and area tables to the folder. preflight --full gains two gates that run both suites when the diff touches modules/dasLLAMA/. The module's CLAUDE.md opens with the four-line workflow.

Where to look: trySerializeProgramModule in src/ast/ast_parse.cpp (stampModuleSource, deriveSourcePolicies, the resume branch), serializeProgramImpl and the two FileInfo read operators in src/builtin/module_builtin_ast_serialize.cpp, select_areas / area_plan / areas_for_path in modules/dasLLAMA/tests/run.das, gate_dasllama in utils/internal/preflight/main.das.

Validation, claims, ledger

Validation

  • Negative controls, each run and restored: with the access route off, test_source_lines_survive_the_cache fails on a leaked LINT003; with mtime stamping restored, the byte-identical-rewrite arm falls back; with lint_check/no_lint off the policy list, the policy test serves the wrong record; with the source-derived syntax off, a -v1syntax host reparses a gen2 module every run.
  • test_chat's five gemma-4 instruct-wire cells were red on this box before the fix (three failing cells observed); the reverse control was not re-run after the fix.
  • The full preflight chain was stopped by decision in its last gate (dasllama-stocked, 18 minutes in, with about 50 minutes of chain wall behind it); every gate before it had completed, but their verdicts died with the process. The dasLLAMA model-free suite had run green inside it (63 files, about 6 minutes, warm caches) and 43 stocked files had passed. The interp and JIT sweeps over tests/ re-ran green afterwards on this box (M5 Max) with the module cache on by default: interp 14179 tests, 14175 passed, 4 skipped; JIT 14045 tests, 14043 passed, 2 skipped.
  • tests/module_cache 9/9, utils/lint 118/118, llvm_cache_selfheal and llvm_ast_roundtrip under -jit, test_run_suites 8/8 (including a runner dry run with a no-op child binary), preflight's own tests 13/13.
  • The new extended_checks row (--test ./utils/internal/preflight/tests) ran on macOS only; the linux cell on this PR's head commit is its evidence for the lane's platform.

Claims - stated, not tested

  • The module takes its own FileInfo from the access (letGoOfFileInfo); without that transfer the module's owner and the access's map both hold the live object, a double free at teardown the functional suite cannot see. An ASAN run of tests/module_cache would pin it.
  • The policy-mismatch cutoff reason is reached only when a record's payload length is unusable (zero or out of range); every test path resumes in place.
  • The two dasllama-* gates' Skip arms (git diff failure; no change under the module) are covered by the pure trigger's test, not by a gate run; the run arm fired on this branch.
  • The record's FileInfo object is not leaked when the live one wins: FileInfo::serialize and TextFileInfo::serialize register it on deleteUponFinish before the swap. An external review round claimed a leak here; the registration is at lines 2178 and 2191.
  • The gemma-4 thinking-on plus tools wire (the <|think|> gate then the <|tool> block in one system turn, bare opener) is what the server renders and has no oracle cell.

Not done

  • dastest compiles each test program at runtime after the host has finished the module cache, so every engine-requiring test file still pays about 14 s of engine compile; keeping the cache installed for the whole process, keyed per test file, is a follow-up and wants a look at why an engine record serializes to about 210 MB.
  • Preflight's 20-minute budget: the chain measured here (make-pr 12.5 min, mostly dupes; preflight about 20 min serial; dasLLAMA suites on top) is the baseline for the rework that follows this PR - parallel lanes, reach sets on module-owned gates, dupes and the stocked suite out of preflight, imgui and sequence as a module category. The dasLLAMA long tests (batch_decode, ple_modes, and the ones that require lcpp_bench by path and pay its compile) are the PR after that.
  • Lint candidates surfaced by the audits: a REVIEW.das gate that every CodeOfPolicies field is on DAS_MODULE_CACHE_POLICY_FIELDS; every Module("<name>") literal under src/builtin in review_nttp.das's require list; the quietCache walker over serializeProgram/serializeProgramImpl; no CMakeLists.txt names a dasLLAMA test; every dasllama/ module in MODULE_AREAS or an explicit core list.
  • Rulings owed: C++ folders cannot carry an [arch] citation, so REVIEW_COMMON.md's move-a-fact rule has no compliant outcome there; modules/dasLLAMA/tests/CLAUDE.md is a 700-line architecture doc outside LINT027 by filename; both dasLLAMA checklists sit at exactly 300 lines.
  • The image suite's gemma arm belongs to no area (its name is a prefix of every other gemma* arm); --suite image --arm gemma reaches it.

🤖 Generated with Claude Code

borisbat and others added 19 commits September 4, 2026 14:02
…FileAccess, and a record written under other policies is never served

The cache record carries a file's name and length, never its text. The reader resolves the name through the FileAccess the compile runs with - file system, .das_project hook or in-memory access - and the module takes its own file from the access as a parse does, so a lint reading a nolint: marker off a served module sees the bytes the access serves. The fopen reload in TextFileInfo::getSourceAndLength is gone.

A record also carries the CodeOfPolicies of the compile that wrote it; the module-cache rail refuses a record whose policies differ (verdict: compile policies changed) and rewrites the cache. The policy stream and the compare share one field list, which now carries lint_check and no_lint (stream version 201).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…match reparses in place

The record header carried the file's mtime and size, read by stat on the file name. An access that serves a file from anywhere but that disk path answers -1/-1 on every run, so the header always matched and a changed file was served stale. The stamp is now the size and hash of the bytes the compile's FileAccess serves for the name - no stat - so any access is covered, and a rewrite that leaves the bytes alone serves while a same-size edit inside the mtime's one-second grain does not. Stream version 202.

A record written under other policies now fails the way a damaged one does: reparsed in place and repaired by the writeback, which carries the new policies. As a cutoff it let corruption landing on the policy bytes cut the stream, which the self-heal test caught.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he runner takes --exclude, and preflight --full runs both when the module changed

The model-free suite held every suite-less file, model-gated cells included; on a box with models stocked it was 47 minutes, a fifth of it one PLE file. The list splits by what a file reaches: a file that calls models_dir, model_available, llama2c_dir or whisper_dir is `stocked`, the per-PR model coverage and a run of skips on a bare box; every other suite-less file is `model-free`, the every-change gate. test_run_suites.das reads each listed file and fails a misfiled one, so the split cannot drift. `--exclude <substr,...>` drops files loudly - `--suite stocked --exclude test_ple_modes` is the iteration form.

Preflight gains two full-tier gates, dasllama-model-free and dasllama-stocked, each skipping unless the diff against the base touches modules/dasLLAMA/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
HOW_TO_ADD_A_FORMAT, the kernel parity and unquirk plans and the JIT compile-time ledger spelled the fast stack as -module-cache <path> plus --jit-split-modules=-1; both are what a plain -jit run does now, and the one-unit form is --jit-split-modules=0 or the released -exe. The tests README names the two per-PR suites.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…us test says so

Twelve test files sat in no suite, so no gate ever ran them; the census cell now walks the folder and fails an unlisted file (the CPU-prefill tripwire is the one declared exemption). test_dlim_wipe, test_gpu_tier and test_vad reach no fixture root and join model-free; test_batch_decode, test_chat, test_facade, test_fused_decode, test_gpu_slot_swap, test_kv_codec, test_kv_paged, test_mtp and test_sampling load models and join stocked. Eleven pass; test_chat's three gemma-4 cells (mid-conversation set_thinking, the tool wire) are red on this box and stay listed - a gate that hides a red is not a gate.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ys what is still owed

Someone working on audio, vision, TTS, the LLM engine or the tooling runs one command after an edit. Every model-free and stocked file carries an area (audio | vision | tts | llm | infra; a shared helper's test sits in two), each tower area owns its arms of the image suite, and `--area <a,b>` runs the union. `--changed` derives the areas from the diff against --base plus the module's untracked files: a test file maps to its own areas, a tests/_ fixture to the areas it serves, an engine module through MODULE_AREAS (no row = core = every area, the safe default), documents and the runner to infra, performance/ and harness/ to infra, benchmarks/ to llm; each mapping prints, a changed model-suite file prints the --arm hint instead. The run closes with REPORT (files, failures, cells a missing model skipped, files excluded) and NEXT (the per-PR suites still owed; the model suites' --arm form when llm ran). The census test holds the area tables to the model-free + stocked census and pins the path map. modules/dasLLAMA/CLAUDE.md opens with the four-line workflow, which loads the moment a session touches the module.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…xplicitly

gemma-4 defaults to thinking (095827f), so a chat that never calls set_thinking(false) renders the <|think|> gate on its first system turn and arms thinking for the session. Five chats in test_chat - the mid-conversation set_thinking(true) scenario and the four tool-wire chats - pin the INSTRUCT wire (the closed empty thought, no gate) and were written for the old default; they went red the day the default flipped and no suite ran the file. Each now opts out where it starts, which is what its expected stream says.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Per touched root, the diff's added comments were classified and landed: two rules into modules/dasLLAMA/tests/REVIEW.md (run.das stays [export] main only; an instruct-wire cell opts out of thinking first), one into src/builtin/REVIEW.md (a new CodeOfPolicies field joins the cache's policy list), the runner-cache fact into tests/CLAUDE.md, the file charter of test_default_cache_path into tests/module_cache/ARCHITECTURE.md; site-local keepers compressed to one line, restatements of the documents dropped. any_path_under became has_path_under, the house word for a containment scan.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…and the cache checklist names both arms

The parse derives version_2_syntax from the source (`options gen2`) and writes it into the program's policies, so a record carried the derived value while the compare saw the caller's: a -v1syntax host, or a file opting out of gen2, mismatched every warm run and rewrote the cache forever. The read now derives the same flag from the source the access serves before comparing. The module-cache checklist splits its one fused rule into the default-path silence rule, stated as the property rather than a stale list, and the explicit-cache verdict rule.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…checklists say what they mean

Ten policy fields sat outside DAS_MODULE_CACHE_POLICY_FIELDS, so the compare could not see them; the list now carries the whole struct (stream version 203) and src/builtin/REVIEW.md states the list as absolute, splits its fused rule into the same-change duty and the standing ban, broadens the version-bump trigger to any change of what the file streams, and states the quietCache rule as the property. src/ast/REVIEW.md gains the rule that a read diagnostic leaving trySerializeProgramModule takes REVIEW.das's scan with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ch suite a file sits in

Sixteen CLAUDE.md paragraphs still opened with "model-free" for files the split moved to stocked; they now say stocked. The README shows the bare stocked run as the PR's, the excluded form as the iteration form, and the direct dastest form under -jit. The checklist lifts "no --exclude on a PR's stocked run" to its own rule, binds the runner's on-require rule to side effects rather than syntax (the data tables the tests read are the point), binds the CLAUDE.md duty to the clause that names a moved gate, collapses the three rules the census test enforces into "weakening test_run_suites.das is a defect", adds that test to the pinned set, states the pinned-gate rule as coverage never shrinking, and names area_image_arms in the arm-census rule. The MODULE_AREAS duty moves to the module's own checklist, where a module-adding diff meets it. The census test gains the reverse check: every arm the image suite declares is reachable from an area or listed as unclaimed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the utils checklist narrows its record-the-run rule

utils/internal/preflight/tests had no CI row, so the trigger predicate of the two dasLLAMA gates was covered by a test nothing would run again; the row beside the review-md and make-pr tests runs the directory. The utils checklist's record-the-run rule now fires only for assertions no CI row CAN run, so a missing row is one finding, not two.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s walls instead of restating old ones

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…opens it, and the serializer checklist binds by function

The same-change duty for a new CodeOfPolicies field moves to include/daScript/simulate/REVIEW.md - the folder walk opens a checklist only for a folder the diff touches, so in src/builtin it could fire only after the duty was met. The src/builtin rule keeps the ban in its positive form (put the field on the list) and the quietCache rule names serializeProgram and serializeProgramImpl and covers dropping a gate from an existing line.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…arries, and the census test pins the runner's no-[init] contract

The "weakening test_run_suites.das" rule duplicated the pinned-set row and restated the test's own asserts; it goes, and CLAUDE.md states the split criterion directly. The per-PR head reads one way: both suites, the stocked one on a box with the models. The arm-census rule drops the image-arm half the census test enforces. Four ragged paragraphs reflowed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nners and contracts get tests, and the documents say exactly what the code does

--changed against an unfetched base read as "nothing changed" and exited 0; a base git cannot resolve is now refused. The runner's pure planners, argument contracts and end-to-end dry run (a no-op child binary) are tested, and the census test pins that run.das carries no [init]. The REPORT line names every skip cause, not only a missing model. modules/dasLLAMA/CLAUDE.md's workflow says what a model-suite file does under --changed and that a PR's stocked run carries no --exclude; the HOW_TO index gains HOW_TO_GET_SIDECAR.md and drops its planned-entries line; tests/CLAUDE.md labels every stocked file, states the --changed map as the code has it, and renames its per-PR section (the checklist's citation follows); the module checklist binds the flag surface to both CLAUDE.md copies. The preflight skill stops claiming review-md has no CI lane, spells the -jit form and the failure-only output, and drops two census numbers; build_and_debug splits the invalidation sentence into its FileAccess and path-read arms, names the policy mismatch as a partial cause, and states the AOT emitter's home in the present tense.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…neteen more fields, stream version 204

The earlier completion pass enumerated the struct up to a nested closing brace and missed nineteen fields, force_escape_free and the gc_infer_collect knobs among them - a compile that flipped one was served the other's AST. A brace-matched enumeration against the list is now empty both ways.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the dasLLAMA checklist stays under the 300-line gate

The MODULE_AREAS duty binds a module whose changes reach some areas but not all, llm included; the flag-surface duty moves to the tests checklist where run.das lives, naming the two document sections as they are headed; the module-cache rules drop the census parentheticals for the property; the utils record-the-run rule shares its sibling's trigger and puts the build the diff produces in the duty; the src/ast rule's WHY loses its pun.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…checklist's rule spends no spare words

The two overlapping gate-step rules merge into one on the property (weakening a per-PR gate step, however spelled), the term is defined once, and the evidence rule names the command the diff adds or changes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 5, 2026 02:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes core cache/serialization behavior and stream format while also reshaping test gating and workflows, warranting careful human validation across platforms and usage modes.

Pull request overview

This PR reworks the front-end (AST) module cache to serve module sources through the active FileAccess and to validate cache records using content stamps and compile-policy stamps, improving correctness for non-disk and hook-based file systems. It also restructures dasLLAMA’s per-PR testing workflow (suite split + areas) and wires corresponding local/CI gates and documentation updates.

Changes:

  • Module cache: stamp module source by content (size+hash) rather than mtime, resolve FileInfo through FileAccess, and refuse serving records written under different CodeOfPolicies (stream version bump).
  • Tests/docs: extend module-cache test coverage for policy mismatch, content stamping, and source-derived syntax policy; update architecture/review docs accordingly.
  • dasLLAMA: split per-PR coverage into model-free vs stocked, add --exclude, --area, --changed runner support, and add preflight/CI hooks plus census tests to keep tables consistent.
File summaries
File Description
utils/REVIEW.md Clarifies the rule for recording non-CI-runnable assertions’ executed runs in the PR description.
utils/internal/preflight/tests/test_changed_set.das Adds unit coverage for has_path_under (diff-scoped gating helper).
utils/internal/preflight/main.das Adds diff-scoped dasLLAMA gates (model-free, stocked) to full preflight.
utils/internal/preflight/config.das Introduces has_path_under helper used by preflight gate triggers.
tests/module_cache/test_default_cache_path.das Adds explicit-cache tests for policy mismatch, content stamping, and source-derived syntax policy behavior.
tests/module_cache/REVIEW.md Splits and strengthens checklist bullets for default-path silence vs explicit-cache verdict checks.
tests/module_cache/ARCHITECTURE.md Documents new record stamps (policies + source content) and expected invalidation semantics.
tests/jit_tests/llvm_cache_selfheal.das Updates comment to reflect new record-header layout (hash+size+deps).
tests/jit_tests/llvm_ast_roundtrip.das Updates comment to reflect content-based invalidation rather than mtime-based.
src/simulate/debug_info.cpp Removes on-demand disk reload from TextFileInfo::getSourceAndLength (source now expected via FileAccess).
src/builtin/REVIEW.md Tightens rules around serializer versioning, policy-field handling, and quietCache gating expectations.
src/builtin/module_builtin_ast_serialize.cpp Implements FileInfo resolution via FileAccess, introduces full policy-field streaming list, and enforces policy-match on read.
src/ast/REVIEW.md Adds checklist rule ensuring diagnostics moved out of trySerializeProgramModule remain gated by REVIEW.das.
src/ast/ast_parse.cpp Switches cache record header stamping to content hash/size; derives syntax policy from source; logs/handles policy mismatch as resumable partial.
src/ast/ARCHITECTURE.md Updates module-cache architecture narrative for content stamping, FileAccess sourcing, and policy mismatch semantics.
skills/internal/preflight.md Documents new dasLLAMA full-tier gates and refines preflight documentation structure.
skills/internal/build_and_debug.md Updates module-cache documentation to match content-stamp + policy-stamp behavior.
plans/unquirk_pass.md Updates dev-loop notes to reflect defaults (module cache + split JIT).
plans/kernel_parity_pass.md Updates dev-loop invocation to default behaviors and how to force monolith.
plans/jit_compile_time.md Updates recommended invocation and clarifies default module cache + split-module JIT behavior.
modules/dasLLAMA/tests/test_run_suites.das Adds census tests to enforce suite split + area-table integrity + runner contract behavior.
modules/dasLLAMA/tests/test_chat.das Opts gemma-4 instruct-wire tests out of thinking-by-default (set_thinking(false)).
modules/dasLLAMA/tests/run.das Adds stocked suite, --exclude, --area, --changed planning; adds REPORT/NEXT output; refactors runner orchestration.
modules/dasLLAMA/tests/REVIEW.md Updates testing discipline to include stocked, runner contracts, and thinking-wire opt-out rule.
modules/dasLLAMA/tests/README.md Documents runner invocations for new suites and --changed.
modules/dasLLAMA/tests/CLAUDE.md Updates workflow and suite/area semantics documentation; explains --exclude, REPORT/NEXT, and preflight integration.
modules/dasLLAMA/REVIEW.md Adds rule requiring new dasllama/ modules to be mapped into MODULE_AREAS when applicable.
modules/dasLLAMA/HOW_TO_ADD_A_FORMAT.md Updates dev-loop guidance to reflect default caching/split-JIT behavior and knobs.
modules/dasLLAMA/CLAUDE.md Adds top-level “Test workflow” summary aligned with new runner semantics.
include/daScript/simulate/REVIEW.md Adds checklist requirement tying CodeOfPolicies changes to the module-cache policy-field list.
include/daScript/ast/ast_serializer.h Adds AstSerializer fields (fileAccess, policyMismatch), updates parsedModules stamp fields, bumps stream version to 204.
.github/workflows/REVIEW.md Consolidates workflow-step weakening guidance and clarifies evidence expectations for workflow gate edits.
.github/workflows/extended_checks.yml Adds CI step to run utils/internal/preflight/tests under extended checks.
Review details
  • Files reviewed: 33/33 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@borisbat
borisbat merged commit 9586461 into master Sep 5, 2026
38 checks passed
@borisbat
borisbat deleted the bbatkin/module-cache-followup branch September 5, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants