Real-ARA no-panic regression coverage (#3) - #4
Merged
Conversation
Draft plan for vendored ara-paperbench subset (hermetic CI check) plus an opt-in submodule full-sweep test, asserting the parser never panics and always produces a ParseReport on real corpus artifacts.
Clarify that pivot coverage stays submodule-only until ARA-Demo's LICENSE actually merges upstream; do not vendor all-rights-reserved files on assumption. Add a Fast-follow section to vendor nanogpt_ara (bringing pivot into the hermetic check) once the license lands.
Narrow the no-panic contract to "no unwinding panic + report produced" (catch_unwind misses stack-overflow SIGABRT), add DRY test harness with a count guard, an always-on sweep-gate unit test, publish exclude for the CC-BY corpus, and a verify-before-vendor step. Resolve open questions Q1-Q3 and file T-PARSE-DEPTH for parser recursion hardening.
Plan step 1/2: Vendor the subset, verify each candidate under parse_dir, and record verified drift outcomes in corpus/SOURCE.md. Vendors 6 ara-paperbench artifacts (CC-BY-4.0, pinned 3fe7ab4) into crates/ara-core/tests/fixtures/corpus/ via scripts/vendor-corpus.sh, keeping only trace/exploration_tree.yaml + logic/claims.md. Each was run through 'ara validate' to confirm no panic and capture the observed Ok/Err outcome + diagnostic counts, folded into SOURCE.md's drift table. Verification corrected the predicted drift: sample-specific-masks covers real cycles (not broken evidence refs); nanogpt-speedrun's 29 errors are the broken evidence:-ref dimension.
Plan step 2/2: corpus_no_panic.rs with shared discover_artifacts + assert_parses_without_panic helpers. - vendored_corpus_never_panics: always-on, walks tests/fixtures/corpus, asserts >= 6 artifacts (closes the vacuous-pass hole) then that each parses without an unwinding panic. - full_corpus_sweep_never_panics: #[ignore] + RUN_CORPUS_SWEEP=1 gated sweep over corpus-external/ submodules; skips cleanly (logging why) when disabled or absent so a fresh clone passes. - should_run_sweep: pure gate fn; sweep_gate_logic unit-tests all 4 cases always-on, guarding the fresh-clone-passes invariant in CI. Documents the no-unwinding-panic scope (SIGABRT/hang out of scope, T-PARSE-DEPTH) and the panic=abort dependency.
Plan step 4: git submodule add for the two upstream corpora under corpus-external/, one per repo for independent pinning. - ara-paperbench (CC-BY-4.0) pinned 3fe7ab4 - ara-demo (no license) pinned 8f18471 — pointer only, never vendored .gitmodules documents the pins, the no-default-checkout contract, and shallow=true for cheap init. Sweep over all 34 artifacts passes (RUN_CORPUS_SWEEP=1 --ignored).
Plan steps 6-8. - CONTRIBUTING.md: 'Running the full corpus sweep' section (submodule init + RUN_CORPUS_SWEEP=1). - ci.yml: explicit comment that the test job must NOT init submodules. - ara-core Cargo.toml: exclude=['tests/fixtures/**'] so cargo publish never ships CC-BY corpus / MIT official data in the MPL-2.0 tarball. Verified: cargo package --list shows 0 tests/fixtures entries.
Plan steps 9-10. - Workspace version 0.0.2 -> 0.0.3; ara-cli's ara-core dep pin bumped to match; Cargo.lock refreshed (version strings only, no dep churn). - CHANGELOG Added entry for the no-panic regression coverage. - TODOS: note under T-REAL-CORPUS that the no-panic net now exists and is distinct from widening (T-PARSE-DEPTH already tracked).
Integration fixes from spec + code-quality review of the harness: - collect(): skip symlinked dirs via file_type() so a symlink cycle in the corpus-external sweep can't stack-overflow (an abort catch_unwind cannot catch — the exact failure mode this net guards against). - Gate repo_root/discover_artifacts/collect and the std::path import on feature="native" so --no-default-features stays -D warnings clean. - vendor-corpus.sh: let git stderr through (keep >/dev/null for progress) so a clone/fetch failure under set -e is diagnosable. All findings were Low severity; sweep_gate_logic + the count guards were confirmed correct as-is. Full suite + sweep (34 artifacts) still green.
EYH0602
added a commit
that referenced
this pull request
Jul 12, 2026
All 6 plans in plans/ were for features that have shipped, so per CLAUDE.md's plan→docs lifecycle they move to docs/ as design records and out of plans/. - New design docs: stage-0-ci-tooling, stage-2-dag-layout, real-corpus-no-panic - Stage 0 (#1), Stage 2 (#5), issue #3 (#4) had no docs/ counterpart → written - issue #9 layout modes already folded into docs/stage-3-viewer.md → just removed - ara-runtime-impl-plan + stage-overview were meta-planning artifacts fully realized in code + per-stage docs → removed - Retire completed TODOs T-DOCS (this migration) and T-VIEWER-DIST-PACKAGING (resolved by the Stage 4 include_dir! embed), and the obsolete T-STAGE4-VERSION-BUMP (Stage 3/4 both shipped) - Fix dangling plan references in README.md and TODOS.md T-MSRV
EYH0602
added a commit
that referenced
this pull request
Jul 12, 2026
) * docs(plans): retire shipped issue-7 plan, refresh Stage 5 hub-deploy plan Remove plans/issue-7-tree-list-mode.md — shipped in #14 (0.1.2) and folded into docs/stage-3-viewer.md + docs/manifest-schema.md. Refresh Stage 5 plan against the shipped Stage 4 code: correct the version target (0.1.2 -> 0.1.3), resolve hub routing (path-based /a/{id}/ with document-base-relative viewer URLs) and Docker assets (embedded-only), drop the now-moot precompressed-assets step, and add a --host flag + broken-ARA skip-on-ingest. * docs(plans): fold Stage 5 eng review findings into hub-deploy plan Eng review (14 findings + outside voice) folded into the Stage 5 plan: Docker needs no wasm toolchain (bundle committed + baked via include_dir!), harden base-href splice + id charset, no SPA fallback at hub root, share serve_cached_manifest, define empty/bad-root startup, clap ArgGroup for mode select, 308 (not cacheable 301) for known ids only, drop unused parsed Manifest on the hub path, wire --assets end-to-end, add wasm + headless browser tests for D1. Defer per-ARA figures (ServeDir cannot nest under a {id} param) and record the static-export alternative (D3: keep server). Add T-HUB-FIGURES and T-STATIC-EXPORT to TODOS.md. * feat(viewer): resolve API/live URLs relative to document base Plan step 1/4: Viewer relative API/WS URLs (T5) Make ManifestSource default URLs relative (api/manifest, api/live) so they resolve against document.baseURI — local serve unchanged (page at /), hub serves under <base href="/a/{id}/">. Refactor the ws:// scheme-swap core into a pure, wasm-testable ws_url_from_base(base, path). Native test guards the relative defaults; wasm test proves resolution for root and /a/{id}/ bases. Note: viewer source changed -> embed regen follows in step 4. * feat(serve): hub ingest cache — parse-once, immutable, id-guarded Plan step 2/4: Hub cache ingest (T2, T8, T12) Add serve/hub.rs: ingest(root) scans immediate child dirs into an immutable Arc<HashMap<id, Arc<CachedAra>>> (lock-free reads, no watcher). Ids constrained to [A-Za-z0-9._-]+ (rejects spaces/non-ASCII/slash/..) — one guard covering path-segment, URL, and HTML-escape safety. Broken ARAs and duplicate ids are logged/skipped, never fatal; an unreadable root is fatal (Err). Per-entry read errors are skipped, not dropped. cache.rs: manifest field is now Option<Arc<Manifest>>; from_dir_lean() drops the parsed graph the hub never reads (~2x resident memory saved per ARA) while from_dir() keeps it for local serve. Shared build() produces identical json + etag + figures_dir either way. Native tests cover: two good, one broken skipped, empty->empty, missing->Err, rejected charset, duplicate id kept-first, lean drops manifest. * feat(serve): hub router + CLI mode selection Plan step 3/4: Hub routing + CLI args (T3, T4, T7, T9, T10, T11, T13) - clap ArgGroup models local (positional dir) vs hub (--hub --ara-root) so bad combos fail at parse time; add --host (default 127.0.0.1, honors 0.0.0.0 for containers) (T9). - Extract serve_cached_manifest(&CachedAra, &HeaderMap) shared by the local and per-ARA hub manifest handlers — one conditional-GET source of truth (T7). - build_hub_router: /a/{id}/api/manifest (404 if unknown), /a/{id}/ index with <base href> injected + no-cache, / index listing, strict shared-asset fallback. No /api/live, no watcher. - Base-href splice guarded: no <head> -> 500, never a silent base-less page (T3). - Root asset serves real files only, else 404 — no SPA fallback (T4). - Bare /a/{id}: known -> 308, unknown -> 404 (never 308-to-404) (T10). - --assets wired end-to-end in hub (on-disk index + shared ServeDir) (T13). - hub oneshot tests incl. two sequential reads = same etag (pure cache hit, T11) + full parse-arg matrix; also applies pending rustfmt to cache.rs/hub.rs. run() branches to hub when --hub is set; local path behavior unchanged. * build: Docker image, deploy docs, CI smoke test, embed regen, 0.1.3 Plan step 4/4: Regen embed + Dockerfile + CI + docs (T1, T6, T14) - Regenerate the embedded viewer bundle (step 1 changed viewer source, so viewer-embed-fresh required a regen). - Dockerfile: multi-stage musl -> distroless, cargo-chef dep cache, NO wasm toolchain (viewer baked in via include_dir!). Pins the repo toolchain before adding the musl target and points CC/linker at musl-gcc. Verified: image builds, 3.1 MB compressed (< 20 MB budget) (T1). - .dockerignore keeps the context small (target/, corpus, git, plans/docs). - docs/deploy.md: hub routing, ingest, Docker/compose, systemd, Caddy/nginx reverse-proxy compression, and the manifest.json-fallback-inert-on-hub note (T14). Folds the plan into docs per CLAUDE.md. - CI: docker job (build + image-size budget + container smoke test: manifest 200/etag/304, /a/{id}/ html+base, wasm application/wasm). - viewer wasm test: relative api/manifest resolves under <base href="/a/x/"> in a real browser (T6). - Bump workspace 0.1.2 -> 0.1.3 + ara-core pins; CHANGELOG [Unreleased]. * docs: retire stage-5 plan (folded into docs/deploy.md) Plan fully implemented across the 4 preceding commits; per CLAUDE.md the plan is rewritten as a design doc (docs/deploy.md) and removed from plans/. * fix(docker): link the musl binary statically for distroless The CI docker job failed with 'exec /ara: no such file or directory'. The CARGO_TARGET_..._LINKER=musl-gcc override made musl-gcc the link driver, which links dynamically against /lib/ld-musl-x86_64.so.1 — absent from distroless/static, so the loader-less image can't exec the binary. Drop the linker override (rustc's musl target links fully static via its self-contained startup objects) and pin RUSTFLAGS=-C target-feature=+crt-static. Keep only the CC=musl-gcc override, which is what actually fixed the build script's -m64 error. Verified: the extracted binary is static-pie with no interpreter, and the container passes the full smoke test locally. * style(design): FINDING-001/004 — fix WCAG AA contrast for muted + accent text - --muted: #90856f → #726751 (3.38:1 → 5.16:1 on --bg, 3.10:1 → 4.75:1 on --panel2) - Add --accent-text: #8c4414 for text-on-light uses (3.67:1 → 6.60:1 on --bg) - Original --accent preserved for borders/backgrounds where 3:1 suffices * style(design): FINDING-002 — add focus-visible ring to tree-list rows Tree rows are role="button" + tabindex="0" but had no visible keyboard focus indicator. Adds a 2px accent outline matching the graph-mode node focus style. * style(design): FINDING-003 — respect prefers-reduced-motion Disable skeleton shimmer animation when the user's OS has motion reduction enabled. The only animation in the viewer is this loading shimmer. * style(design): FINDING-005/006 — enlarge touch targets, bump body font - Controls: height 1.75rem → min-height 2.25rem (28px → 34-36px) - Body font-size: 14px → 15px (closer to the 16px accessibility guideline while preserving data-dense tool proportions) - Applies to .toolbar-search/.toolbar-select, .layout-toggle, and .btn * style(design): FINDING-007 — use color-mix for --warn strikethrough Replace raw rgba(162, 59, 45, 0.4) with color-mix(in srgb, var(--warn) 40%, transparent) so the decoration color tracks the token if --warn changes. Matches the same technique used in the status pills. * build: regenerate embedded viewer bundle for design fixes The design-review CSS fixes (FINDING-001..007) changed the ara-viewer source, so `ara serve` shipped stale UI when run without --assets. Regenerated via scripts/embed-viewer.sh so the embedded bundle matches the source and CI's viewer-embed-fresh check passes. * docs: fold shipped plans into docs/, remove plans/ All 6 plans in plans/ were for features that have shipped, so per CLAUDE.md's plan→docs lifecycle they move to docs/ as design records and out of plans/. - New design docs: stage-0-ci-tooling, stage-2-dag-layout, real-corpus-no-panic - Stage 0 (#1), Stage 2 (#5), issue #3 (#4) had no docs/ counterpart → written - issue #9 layout modes already folded into docs/stage-3-viewer.md → just removed - ara-runtime-impl-plan + stage-overview were meta-planning artifacts fully realized in code + per-stage docs → removed - Retire completed TODOs T-DOCS (this migration) and T-VIEWER-DIST-PACKAGING (resolved by the Stage 4 include_dir! embed), and the obsolete T-STAGE4-VERSION-BUMP (Stage 3/4 both shipped) - Fix dangling plan references in README.md and TODOS.md T-MSRV * docs: drop dangling plan references in README and TODOS Point README at docs/ instead of the removed master plan, refresh T-MSRV context to the crates.io publish, and retire completed/obsolete TODOs (T-DOCS, T-VIEWER-DIST-PACKAGING, T-STAGE4-VERSION-BUMP).
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
Implements
plans/issue-3-real-corpus-no-panic.md— closes #3.Adds a permanent no-panic regression net for the ARA parser over real corpus data. Real artifacts exercise a wider schema than
ara-coremodels today (extra node fields, transition fields, real cycles, broken evidence refs, anara-2.0streams doc), so this is explicitly not a clean-parse check: it asserts only thatparse_dirnever unwind-panics and always produces aParseReport(bothOkandErroutcomes pass). Robustness, locked in permanently, before Stage 2 freezes the wire schema.Plan Steps
ara-paperbenchsubset +scripts/vendor-corpus.sh; verify each candidate underparse_dir; write verified drift table incorpus/SOURCE.md(149c03e)corpus_no_panic.rsharness — shareddiscover_artifacts+assert_parses_without_panic,>= 6count guard,should_run_sweep+sweep_gate_logic(441193e)corpus-external/submodules (ara-paperbench, ara-demo) (9b50a5e)exclude = ["tests/fixtures/**"](1eeb6b3)0.0.2 → 0.0.3, CHANGELOG, TODOS close-loop (4e97454)native, unmute git in vendor script (c7f01dc)Verification correction (step 1.5 paid off)
Running each vendored artifact through
parse_dirbefore locking it in as the acceptance bar corrected the plan's predicted drift:paperbench/sample-specific-masksactually covers the real-cycle dimension (2 cycle errors), whilespeedrun/nanogpt-speedrun's 29 errors are the brokenevidence:claim-ref dimension.corpus/SOURCE.md's drift table records the reproduced-and-verified outcomes.Review Status
Eng review CLEAR (PLAN) via
/plan-eng-review(VERDICT: ENG CLEARED; all 6 findings folded into the plan before implementation). Post-implementation, the test harness passed an independent two-stage subagent review: SPEC PASS (all 14 requirements, drift table independently reproduced) and QUALITY PASS (4 Low findings, 3 applied in c7f01dc, none blocking).Test Plan
cargo test --workspace --lockedgreen — newvendored_corpus_never_panics+sweep_gate_logicpass,full_corpus_sweep_never_panicscorrectlyignored; existingofficial_fixtures_are_clean+ snapshots unchanged.cargo fmt --all --check,cargo clippy --workspace --all-targets --locked -- -D warningsclean (also verified--no-default-features).cargo build -p ara-core -p ara-wasm --target wasm32-unknown-unknown --lockedgreen (fixtures are test-only,native-gated).sweep_gate_logic).git submodule update --init && RUN_CORPUS_SWEEP=1 cargo test -p ara-core -- --ignoredsweeps all 34 artifacts, no panics.cargo package -p ara-core --listshows notests/fixtures/entries (CC-BY corpus stays out of the tarball).Follow-up:
T-PARSE-DEPTH(recursion depth guard so a pathologically deep artifact yields a cleanErrinstead of a stack-overflow abort) tracked inTODOS.md, out of scope here.