test: make the suites capable of failing — and fix the 19 lexer defects that exposed - #8
Conversation
Foundation work before any axiom is discharged: nothing in this repo could be verified by running it, so every proof-debt item was unfalsifiable in practice. Four layers of the base were soft, and each hid the next: 1. `lake test` reported "no test driver configured" — no driver existed. 2. The suites printed "FAIL"/"✗" and then printed "All tests passed!" unconditionally, with `main : IO Unit`, so the process always exited 0. LexerTest even documented a counter — "Count of test failures, tracked via IO.Ref" — that was never implemented. 3. `test/TypeSafetyTests.lean` was declared by NO Lake target, so it was never built. It had rotted and no longer compiled. 4. `lake build` only builds the default target (the `GqlDt` library), so the test executables were never compiled by CI either. `lexer_test` had also rotted. Changes: * `test/TestHarness.lean` (new) — the failure counter LexerTest's comment promised, plus `summarise`, which turns the tally into a process exit code. * All four suites now record failures and return `IO UInt32`. ParserTest's 18 `✗` sites were printing to stdout and returning Unit; they now record. The unconditional "All tests passed!" banners are gone. * `lakefile.lean` — added `lean_lib TestSupport` (so the suites can import the harness), `lean_exe type_safety_test` (previously unbuildable), and a `@[test_driver] script test` that runs the Lean suites and aggregates exit codes. ffi_test is deliberately excluded: it links liblith_bridge.a and is covered by the zig-ffi CI job, so including it would make `lake test` fail on a clean checkout for a non-Lean reason. Compile fixes needed to get the rotted suites building at all: * LexerTest: `String.containsSubstr` does not exist in Lean 4.15. All three probes test single characters, so `String.contains` is the right primitive. * TypeSafetyTests: `Prompt`/`Provenance` were not opened; `insertEvidence` takes a `Provenance.Rationale`, not a `NonEmptyString`; `BoundedNat`'s min/max are structure PARAMETERS, not fields, so `BoundedNat.mk 0 100 100 …` passed bounds as data — replaced with anonymous constructors that let the expected type supply them. Added a top-level `main` alias, since the suite's `main` sits inside a namespace and the linker found no entry point. Result: `lake test` runs, exits 1, and reports 19 REAL failures in the lexer that were previously invisible — multi-character operators (`<=`, `>=`, `!=`, `<>`) never lex as single tokens, `:` lexes as `::`, a bare `-` produces no token at all, and block comments are unimplemented (`/* b */` lexes as `/ * b * /`). Parser and TypeSafety suites pass. Those 19 are NOT fixed here — this commit only makes them visible. The README's "✅ Lexer: ... operators, literals, comments" claim is now known to be false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`LexerState.peek` takes an offset whose default is 1 and where **0 means the
CURRENT character** — `s.peek 0` is exactly `s.curr`. All seven two-character
lookahead sites passed 0, so every one compared the character it already held
against the character it expected next. The branches were all present and
correct; none could ever be taken.
Consequences, all of which `spec/GQL-DT-Lexical.md` mandates and none of which
worked:
* `<=`, `>=`, `!=`, `<>` (§ operator table, precedence 5) never lexed as single
tokens — `<=` came out as `[opLt, opEq]`.
* `:` lexed as `::`. The lookahead saw its own `:` at offset 0, matched the
double-colon branch, emitted `opDoubleColon` and advanced TWICE. The spec
makes these distinct: `::` is cons (prec 6), `:` is type annotation.
* A bare `-` produced NO token: `skipWhitespaceAndComments` matched
(curr, peek 0) = ('-', '-') and treated a single minus as the start of a line
comment, swallowing the rest of the input.
* §8.2 C-style block comments were never recognised — `/* b */` lexed as
`/ * b * /`.
Fix is `peek 0` → `peek 1` at all seven sites.
Also corrects one genuine TEST bug this exposed: `schema::table` was expected to
yield three identifiers, but `table` is a reserved SQL keyword and keywords are
case-insensitive, so it correctly lexes as `.kwTable` — as this same suite
asserts under "SQL Keywords". The qualified-identifier case now uses a
non-reserved name, and the keyword interaction is asserted explicitly rather
than left as a latent contradiction between two tests.
`lake test`: 19 failures → 0. 163 checks across Lexer/Parser/TypeSafety, exit 0.
Canary-tested both directions, because a gate that has never gone red is not
evidence of anything: seeding `firstType "SELECT" == some .kwDelete` turns
`lake test` red (exit 1, "Lexer: 1 check(s) FAILED"); removing it returns exit 0.
READMEs updated. The "✅ Lexer: ... operators, literals, comments" claim was
false when written and is now true and verified rather than asserted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-adds the five C-A-G-M files proposed by the unmerged `sweep4` commit
("Generated by Mistral Vibe"), which was the only one of the three sweeps whose
content was not already superseded. Not replayed — rewritten, because the sweep
version had two defects:
* `.github/CODEOWNERS` and `.github/funding.yml` named @metadatastician, which
owns other estate repositories but not this one. Ownership here is
@hyperpolymath.
* `ARCHITECTURE.md` was generic boilerplate describing a `src/ tests/ config/`
layout with "modular, maintainable architecture designed for clarity,
scalability and long-term sustainability". This repository has none of those
directories. It would have actively misdescribed a Lean 4 + Zig + Idris2
project — worse than having no file.
What they say instead:
* ARCHITECTURE.md — the real two-layer structure (GNPL lowers to GQLdt lowers to
the Zig bridge), the actual directory map, the build ORDER (the Zig archive
must exist before Lean links, which is what the Containerfile used to get
backwards), and a verification-posture table naming each gate and what it does
and does not establish.
* GOVERNANCE.md — the rules a change must clear, each traced to a specific past
failure rather than asserted as principle: no handwaving, gates must be shown
to go red, specs are normative, the trusted base is enumerated, foundation
before depth. Also records the two cross-cutting surfaces that cannot be
changed on one side only — the FFI boundary, and PROMPT scoring, where the
averaging rule is welded into a proof field.
* MAINTAINERS, CODEOWNERS, funding.yml — correct owner; CODEOWNERS additionally
calls out the proof surface, the ABI and docs/proof-debt.md.
The sweep commits remain on `backup/sweeps-mistral-vibe`; nothing is lost.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review ✅ Approved 1 resolved / 1 findingsAdds executable test suites and fixes 19 off-by-one lexer lookahead defects, but the ✅ 1 resolved✅ Bug: lake test driver never builds the test executables
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
CI failed at "Run Lean tests" with the driver reporting the suite binaries missing — which was correct, and was the same fault this PR exists to close. `lake build` builds only `@[default_target]`, which was the `GqlDt` library alone. So the test executables were compiled by nothing: not by CI, not by a plain local `lake build`. That is exactly why two of the four suites had rotted to the point of not compiling, and my test driver inherited the problem by expecting binaries that nothing had produced. Marking the three pure-Lean suites `@[default_target]` closes the class, rather than papering over it by adding an explicit build line to the workflow: the suites can no longer silently stop compiling, because every `lake build` compiles them. ffi_test stays non-default — it links liblith_bridge.a, so making it default would break a clean `lake build` before `cd bridge && zig build` has run. Verified from a wiped `.lake/build/bin`: plain `lake build` produces lexer_test, parser_test and type_safety_test; `lake test` then exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Foundation work, breadth-first, before any axiom is discharged — per the standing
"firm foundation, breadth before depth" directive.
The problem: nothing here could be verified by running it
Every proof-debt item was unfalsifiable in practice, because four layers of the base were
soft, and each hid the next:
lake testreported "no test driver configured" — none existed.FAIL/✗and then printed "All tests passed!" unconditionally,with
main : IO Unit, so the process always exited 0. LexerTest even documented acounter — "Count of test failures, tracked via IO.Ref" — that was never implemented.
test/TypeSafetyTests.leanwas declared by no Lake target, so it was never built.It had rotted and no longer compiled.
lake buildonly builds the default target (theGqlDtlibrary), so the testexecutables were never compiled by CI either.
lexer_testhad also rotted.What that was hiding: 19 real lexer defects
Once the suites could fail,
lake testimmediately went red with 19 failures — allgenuine spec-conformance bugs, all from one off-by-one.
LexerState.peektakes an offset where 0 means the current character (s.peek 0≡s.curr; the default is 1). All seven two-character lookahead sites passed0, so everyone compared the character it already held against the character it expected next. The
branches were all present and correct — none could ever be taken.
<=>=!=<>[opLt, opEq]etc.:opDoubleColon(advancing twice)opColon::is cons (prec 6),:is type annotation-opMinus--line comment/* b */[/, *, b, *, /]Fix:
peek 0→peek 1, seven sites. 19 failures → 0.One of the 19 was a genuine test bug rather than a lexer bug:
schema::tableexpectedthree identifiers, but
tableis a reserved SQL keyword and keywords are case-insensitive,so it correctly lexes as
.kwTable— as this same suite asserts elsewhere. Thequalified-identifier case now uses a non-reserved name, and the keyword interaction is
asserted explicitly rather than left as a latent contradiction between two tests.
What landed
test/TestHarness.lean(new) — the failure counter LexerTest's comment promised, plussummarise, which turns the tally into a process exit code. All four suites now recordfailures and return
IO UInt32; ParserTest's 18✗sites were printing to stdout andreturning
Unit, and now record. The unconditional "All tests passed!" banners are gone.lakefile.lean—lean_lib TestSupport(so suites can import the harness),lean_exe type_safety_test(previously unbuildable), and a@[test_driver] script test.ffi_testis deliberately excluded: it linksliblith_bridge.aand is covered by thezig-ffi job, so including it would make
lake testfail on a clean checkout for a non-Leanreason.
Compile fixes needed just to get the rotted suites building:
String.containsSubstrdoesn't exist in Lean 4.15 (all three probes test single characters, so
String.containsis right);
TypeSafetyTestsdidn't openPrompt/Provenance, passed aNonEmptyStringwhere
insertEvidencewants aProvenance.Rationale, and passedBoundedNat's min/max asdata when they are structure parameters — plus its
mainsat inside a namespace, sothe linker found no entry point.
Governance files (your call (b)) — the five
sweep4files, rewritten rather thanreplayed. The sweep version named
@metadatastician(owns other estate repos, not thisone), and its
ARCHITECTURE.mddescribed asrc/ tests/ config/layout this repo does nothave. Replaced with the real structure, the build order, and a verification-posture table.
GOVERNANCE.mdrecords the rules a change must clear, each traced to a specific pastfailure rather than asserted as principle.
Verification
Canary-tested both directions, because a gate that has never gone red is not evidence
of anything: seeding
firstType "SELECT" == some .kwDeleteturnslake testred(exit 1,
Lexer: 1 check(s) FAILED); removing it returns exit 0.READMEs corrected.
✅ Lexer: … operators, literals, commentswas false when written;it is now true, and verified by 163 executable checks rather than asserted.
Not in this PR
flake.nixremoval (your call (c)) is deliberately held. The estate package policy is"Guix primary (
guix.scm), Nix fallback (flake.nix). A repo satisfying neither is theviolation." This repo has no
guix.scm, soflake.nixis currently the only artefactsatisfying it — removing it turns Governance red. Writing a
guix.scminstead would meancommitting a file I cannot execute (guix isn't installed here), which is the handwaving the
doctrine forbids. Raised rather than guessed.
The 16 axioms are untouched. This PR only makes the ground firm enough to stand on.
docs/proof-debt.mdD3 is additionally gated on the arithmetic-vs-geometric mean decision,since
PromptScores.overall_correctwelds the averaging rule into a proof field.🤖 Generated with Claude Code