Repository navigation
review: 2026-09-08 review-and-improvement pass (property laws, runner fail-fast, gate honesty, metrics) - #1
Merged
Conversation
- raise FailFastExit instead of early return so run_batch finally persists the partial manifest, summary, and failed-step evidence on --fail-fast - wire a generous 1800s timeout into every run_cmd_capture call site so a hung child cannot block the batch forever (scan, graph, validate, explain) - repair interrupted git clones: an existing dest without .git is removed before re-clone instead of failing every subsequent run
… coupling - law4/law5/law6 property tests supply complete valid source matrices (the renderer is fail-closed and never invents missing A/B/C/D) - law5 asserts the aggregate A broadcast stays column-stochastic - template contract test accepts a sidecar clone named COGANT
- audit_manuscript_math_adjacency no longer audits unsubstituted text when variable resolution fails: a broken injector/METRICS.yaml is a gate error, not a silent pass - check_coverage_table raises on missing/unparseable METRICS.yaml instead of silently degrading benchmark sidecar comparisons
- REVIEW_LOG_2026-09-08.md documents findings, commits, and verification - TODO.md closes cog-p1-02 (parser capability registry implemented and tested), ticks the resolved 2026-08-02 open items, adds the dated pass section, refreshes Last-reviewed - README/ISA audit batteries invoke audit_stage_list via the inner package env (matching CI); ISA changelog + updated date refreshed
test_count_total=9659, passing=9612, failing=0, skipped=47, coverage=95.55%, mypy_errors=0, roundtrip role_preserved=25/25, strict=1. Generator sha c14e1d9; suite runtime refreshed.
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
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.
Findings → Changes
Package tests. Six property tests pinned a removed contract: laws 4/5/6 (
test_correctness_laws.py) and the matrix-stochasticity laws (test_law_matrix_stochasticity.py) built source models with empty A/B/C/D and expectedrender_matrices_moduleto invent defaults. The renderer is deliberately fail-closed ("no matrix is invented when the source is incomplete"), so the tests contradicted the implemented contract. The other 33 entries in.pytest_cache/lastfailedwere stale cache (config-loader, language-fallback, and package-init clusters all pass at HEAD).→ Strategies now supply complete valid matrices; law4/law5 exercise the degenerate aggregate broadcast (D
[1.0]→ uniform over the declared cardinality; single-column A → column-stochastic expansion) added in 157d3d5. 9612 passed / 0 failed / 47 skipped, coverage 94.61% (gate 89%).run_all orchestration (
tools/run_all_runner.py):--fail-fastreturned mid-loop beforerun_manifest.json/summary.*were written, discarding every completed target's evidence;run_cmd_capturehad no timeout (a hung child blocks the batch forever); an interrupted git clone (dest without.git) poisoned every re-run;export-gnn:{tid}check label mismatched the recordedexport_gnn:{tid}step name.→
FailFastExitaborts after persisting the partial manifest + summary; 1800s timeouts on capture steps; incomplete clones self-repair; label aligned. Verified:./run_all.sh --dry-runexit 0 (24 targets), batch dashboard regeneration exit 0.Gate honesty (
tools/):audit_manuscript_math_adjacencysilently audited unsubstituted text when variable resolution failed;check_coverage_tablesilently degraded benchmark comparisons on missing/broken METRICS.yaml. Both now fail loud.Manuscript. Automated battery green (crossrefs 126 ids/515 refs, citations 155/155, numbers 0 mismatch, links 0, claim scope 0, robustness 8/8, formalisms, synthetic surfaces, module refs 88/0, renderers 14+3). The injected copy under
output/manuscript/was stale vs METRICS.yaml → regenerated via the documented generator; METRICS.yaml regenerated against commit-bound HEAD (passing 9606→9612, failing 6→0, coverage 95.55%, mypy 0, roundtrip 25/25, strict=1); publication readinessverdict=ready, blockers=0.Plans vs implemented state. cog-p1-02 (parser capability registry) is implemented and accepted (
parsers/registry.py+ per-language modules + 13/13 fallback contract tests) → closed in TODO.md; cog-p1-01 stays active (schema-version migration mechanism still partial). Template-contract test de-coupled from checkout directory case (sidecar clone namedCOGANT).REVIEW_LOG follow-through. Of the 2026-08-02 log's three open items: audit-test failure already fixed (5/5), METRICS regeneration redone commit-bound this pass, notebook stubs remain deliberately open. All recorded; new
REVIEW_LOG_2026-09-08.md.Validation
PYO3_USE_ABI3_FORWARD_COMPATIBILITY=1(PyO3 0.22.6 caps at 3.13) rust-check/rust-test/rust-clippy pass; wheel-smoke fails only because no pydantic_core binary supports 3.14. Under the inner 3.12 env the wheel toolchain is supported but the gate's package-tests lane exceeds its 1800s cap (inner addopts add coverage; rust lanes run concurrently). All failures are environmental — this pass touches no Rust, wheel, or dependency code.uv run --directory cogantand never runs this lane).--fail-on-dirtygreen; manuscript numbers 0 mismatch.Known limitation
Template-side
stage_03_render/stage_04_validatecannot resolve--project ongoing/instituteos/repos/COGANT: the untracked intermediate symlinkprojects/ongoing/instituteostrips the template's own committed path-confinement (2dc88a7f6). Remediation is a template-side leaf link (projects/working/cogant), outside this repo.