v1.0.0 readiness assessment, and six gates that could report a pass they had not earned - #156
Open
ChelseaKR wants to merge 12 commits into
Open
v1.0.0 readiness assessment, and six gates that could report a pass they had not earned#156ChelseaKR wants to merge 12 commits into
ChelseaKR wants to merge 12 commits into
Conversation
The `secrets` recipe was one line, under a comment claiming it covered
"working tree + history":
gitleaks detect --source . --redact --exit-code 1
`gitleaks detect --source .` walks commits. It is blind to a file that
has never been added to the index, which is exactly the state a secret is
in at the moment someone is about to commit it, and the moment this gate
is meant to catch.
Measured on this repository, with a root-level file holding an AWS key
pair, a GitHub PAT and a Slack bot token, never staged:
| Command | Result |
|---|---|
| `gitleaks detect --source .` (the gate as it was) | 283 commits scanned, no leaks found, **exit 0** |
| `gitleaks detect --no-git --source .` | leaks found: 1, **exit 1** |
| `--no-git` against the clean tree | no leaks found, exit 0 |
The recipe now runs both scans, each reporting its own PASS/FAIL, and
neither can short-circuit the other -- the same reason `verify` does not
stop at its first failing gate. On the planted tree: exit 2, "gitleaks
working tree: FAIL". On the clean tree: exit 0, both PASS.
`.gitleaks.toml` scopes the working-tree scan away from `.venv/`,
`node_modules/` and build caches. That is not cosmetic: `verify.yml` runs
`uv sync` and `npm ci` before `make verify`, so without it the release
gate scans about 29 MB of third-party code whose test fixtures are a
standing source of findings this project cannot fix. Measured, 28.74 MB
in 2.34s without it against 2.12 MB in 0.30s with it, same verdict on the
planted secret.
`tests/test_secret_scan_gate.py` has five tests. Two read the recipe and
always run, so deleting the working-tree half goes red in every job that
runs pytest. Two drive the real binary, against a planted secret and
against a clean control -- the control is the one that matters, because a
scanner that failed on everything would satisfy the first test without
it. One checks the allowlist covers only dependencies and build output,
so it cannot become a way to stop scanning `src/`.
The planted values are assembled from string fragments rather than
written out whole. As literals the test file was itself a finding
(`leaks found: 1` at `tests/test_secret_scan_gate.py:39`), and the honest
answer to that is an inert file, not an allowlist entry exempting the
file that exists to defend the scan.
Break: against the pre-fix one-line recipe, 2 failed, 3 passed. Restore:
5 passed.
`ci.yml`'s comment is corrected too; it described the job as scanning
"the PR/push diff and full history".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The package had no `py.typed`, here or on `main`. `mypy --strict` runs
over `src/` on every pull request and the v1 contract reserves stability
guarantees for the public exports, and neither of those reached a caller.
A five-line consumer importing `validate_feed` from the installed package
got:
error: Skipping analyzing "tods_validate": module is installed, but
missing library stubs or py.typed marker [import-untyped]
exit 1. With the marker: "Success: no issues found", exit 0.
Freezing a semantic-versioning promise on a public API that every
downstream type checker refuses to look at is a poor thing to do at
v1.0.0, and the fix is one empty file.
Confirmed it ships rather than assumed: `uv build --wheel` produces a
wheel containing `tods_validate/py.typed`. `tests/test_typing_marker.py`
asserts the file exists, that it is inside the package directory, and
that the build includes it.
Break: with the marker removed, all three tests fail. Restore: 3 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`[tool.mypy] files = ["src"]` while `ruff check src tests scripts`
covered all three, so `check_public_contract.py`, `check_npm_audit.py`,
`generate_rules_doc.py` and `spec_watch.py` -- the code behind
`make docs-check`, `make contract-check`, `make npm-audit`,
`make i18n-check` and the weekly spec tripwire -- were the least verified
code in the repository. They are the merge gates. 34 files checked
becomes 44.
This only became cheap once `py.typed` existed, which is why the two
commits are adjacent: 16 of the 17 errors that surfaced were
`import-untyped` against the package's own modules, from scripts
importing `tods_validate`.
The seventeenth was real. `scripts/spec_watch.py`'s
`resp.read().decode("utf-8")` returns `Any`, so the function's `-> str`
was a promise mypy had never been in a position to check.
`tests/` stays out of scope, recorded in `docs/CONFORMANCE-GAPS.md`;
`scripts/` was the half that gates merges.
Break: with the annotation reverted, `make typecheck` exit 2, one error.
Restore: exit 0, 44 files.
Merge note: the same construct is at `origin/main:scripts/spec_watch.py`
line 159, in a file #151 changed heavily. Expect to re-apply this
one-line hunk by hand.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`check_perf_budget.py` timed `run(feed)`, **discarded its result**, and divided an assumed row count (`rows = trips * 2`) by CPU time. A throughput budget can only fire on slowness, so under that arithmetic doing less work reads greener. A validator that had quietly stopped reading the feed would burn almost no CPU, report an enormous rate, and pass *further inside* the budget than a correct one. The gate could not fail for the reason it exists. Nothing noticed, because every existing test stubs `measure` out. Each repetition now counts the rows the loader actually parsed, refuses to report a rate below the floor the generator writes, and refuses when two repetitions of the same feed disagree about the count. The rate's *denominator* is deliberately left as the fixed unit of work. The real count is about 10% higher (110,101 rows for 50,000 trips, not 100,000), and switching would raise every published number by that margin and make the committed baseline look like a speedup nobody made. Also bounded `maxRegressionFactor`, which was read unbounded from the same data file as the baseline. A large enough value there does not loosen the gate, it retires it, and retiring a gate belongs in a reviewed change rather than a data edit. Break: with the three guards removed, 5 tests fail. Restore: 17 passed. End to end on a real 300-trip feed: 761 rows parsed, within budget, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README declared `Observability: Tier C ... Opt-in --log-format json only` and, two paragraphs later, asserted conformance with that tier in the Standards Conformance table. `--log-format` exists nowhere in `src/`, and no row in the gaps ledger recorded that. `OBSERVABILITY-STANDARD.md` §3 defines Tier C as "an opt-in `--log-format json` flag backed by `structlog`". Nothing under `src/` imports `logging` at all, so there is no stream for such a flag to format. In a repository whose entire argument is that it does not claim what it has not checked, this was the worst single thing on the v1.0.0 list. The section now says so and links a new `CONFORMANCE-GAPS.md#observability` row, which sets out both ways to close it -- restate the tier, or add `structlog` as a second runtime dependency to a tool that deliberately has one -- without picking. That choice is a product decision, not a remediation, and it is not on the v1 critical path: `--log-format` is not in `docs/v1-contract-candidate.json`, so adding it later is an additive minor release. What *was* on the critical path was shipping v1.0.0 with the README claiming it. The durable half is `tests/test_readme_claims.py`: every `--flag` the README names must exist in the CLI, unless it is another program's flag (three entries, each attributed) or is recorded as documented-absent (one entry) **and** linked to the gap tracking it. Both allowlists are themselves checked for dead entries, so the claim cannot come back without the flag coming back with it. It also pins the README's derived numbers, 43 rules and 16 `needs_gtfs` rules, against the registry; both check out, and nothing had been comparing them. Two smaller corrections in the same pass. "16 reference checks" became "the 16 checks that read GTFS files", because 6 of the 16 are field, semantic or coverage rules. And the `ingest-ready` paragraph now says it currently resolves to the same settings as `strict`, which it does, byte for byte. Break, against the README and ledger exactly as at HEAD: 2 failed. Restore: 5 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Makefile header enumerates the work CI does that `make verify` cannot, so a contributor knows a green local run is necessary and not sufficient. It listed "the composite action's self-test, CodeQL, Semgrep and zizmor" and omitted two `pull_request` jobs with no `make` equivalent: `perf`, and the VS Code extension `package` job. The second is path-filtered to `editor/vscode/**`, so it is absent from most pull requests, which is how it stayed off the list for as long as it did. Both are named now, and `tests/test_ci_gate_parity.py` pins the promise against the workflows: every job in a `pull_request` workflow either runs a `make verify` gate or is named in the header as something CI does on its own. A job added later cannot reject a tree that `make verify` has just called green without the header saying so. Two parser bugs were found and fixed while writing it, both exactly the kind the test exists to catch. Slicing job bodies with `str.index` over the whole file resolved a job id to an earlier occurrence, so one job's body ran on into the next. And a job's slice ends where the *next* job's leading comment block begins, so the `perf` job looked as though it ran `make a11y`, because the paragraph introducing the accessibility job says so. A check that matched prose instead of commands would have passed while measuring nothing. Break, against the header as it was: `vscode-extension.yml:package` and `ci.yml:perf` both reported. Restore: 3 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`tods_validate.read.to_dataframe` and `tods_validate.__version__` are in `docs/v1-contract-candidate.json` and were named in **none** of the 52 existing test modules. A 90% line-coverage floor cannot see an export that nothing imports, so both were about to be frozen under a stability promise with no test behind them. `to_dataframe` now has its documented no-pandas `ImportError` -- pandas is deliberately not a dev dependency, so that is the path most callers hit first and the one nothing checked -- plus a happy path through a stub module. `__version__` is pinned to `pyproject.toml` and asserted not to be the `0.0.0+unknown` fallback. Underneath both is the floor that finds the next one: every contract export must be named somewhere in the suite. Pointed at the 52 pre-existing modules, `unreferenced_contract_exports()` returns exactly those two names, which is how the gap was measured rather than guessed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`TODS-I601` flags a run spanning more than `_LONG_SPAN_SECONDS = 6 * 3600` with no break event. Its fixture spans eight hours, which proves the rule fires and says nothing about where the bound is. Demonstrated rather than argued: moving the constant from six hours to seven left the conformance test for `TODS-I601` passing, exit 0. The threshold could drift by an hour with the suite green. Two runs one second apart now sit either side of the boundary; against the same seven-hour bound they both fail. The pair also asserts the constant's current value, so moving the bound has to move both sides deliberately rather than turning one of them red by accident. It is the only numeric threshold in the rule set, so the class is closed rather than sampled. Restored: 10 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rency gate `docs/api.md` described `Finding` as 7 fields and 2 helpers. The dataclass has 10 fields and 3 helpers, and `report.schema.json` **requires** the three the page omitted -- `data`, `caused_by`, `severity_original` -- plus `fingerprint()`, which is the identity `--baseline` matches on. A caller working from the page alone could not have read a report this tool writes. Re-verified field by field and re-stamped; the old stamp said "against tods-validate 0.8.0" on a 0.10.0 tree, which the gate could not have caught, because it hashes content and never compares versions. Whether it should is a policy question left for the maintainer, since it would force a re-verification every release. `docs/read-api.md` documents 10 of the 19 contract names and carried no currency stamp at all, so `make docs-check` had nothing to fail on when it drifted. It is in `STAMPED` now, with `FeedFile.readable` and `LoadProblem` documented -- both public, both previously absent, and `problems` was documented without its element type, which left the field unusable from the page alone. `tests/test_doc_currency.py` parametrized over a hand-written list of two paths, a second copy of `STAMPED` that could go stale against it. It now derives from the checker's own `STAMPED`, plus an assertion that the list is not empty -- a parametrize over an empty list reports success without running anything, which is the same failure shape as everything else in this branch. Break: appending one line to `read-api.md` gives `check_doc_currency.py` exit 1, naming the file and the new hash. Restore: exit 0, three files current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One entry per change in this branch, each stating what was wrong, how it was measured, and what is left open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`v1.0.0-readiness.md` is the deliverable: every v1.0.0 checklist item from the four overlapping lists that exist (`docs/roadmap.md`, `docs/v1-contract-audit.md`, `docs/MULTIYEAR-PLAN.md`, `DEFINITION_OF_DONE.md`), assessed against evidence rather than recall, with the verdict **not ready** and the three remaining blockers named. None of the three is code in this repository. It also records where a document is wrong about its own repository, which is the part a reader cannot reconstruct: `DEFINITION_OF_DONE.md` and `docs/v1-contract-audit.md` both say no ruleset is live, and one is, under a different name and with five fewer required checks. `improvement-plan.md` is committed alongside it because the assessment cites it for provenance -- which findings were re-checked against `origin/main` before being called defects, and which two fail-opens turned out to be already fixed by #151 and were therefore not re-fixed. An assessment whose provenance note resolves to nothing is worth less. Both files were written as an uncommitted working record and say so in their own text; those sentences describe the pass that produced them, not this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rebasing onto main brought in a README whose Development section installs with `pip install -e . --group dev` (PEP 735 dependency groups, CQ-27). This test arrived on the branch and had never seen that line, so it read `--group` as a tods-validate flag the CLI does not accept and failed. It is pip's: `pip install --help` lists `--group <[path:]group> Install a named dependency-group from a pyproject.toml`, and the README's use is a `pip install` invocation. Recorded in _FOREIGN_FLAGS with whose flag it is, exactly as --no-deps and --require-hashes already are, rather than widening the pattern that finds flags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ChelseaKR
force-pushed
the
fix/v1-readiness-gate-repairs
branch
from
August 29, 2026 00:57
3c21d60 to
adc5cdb
Compare
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.
An audit pass against the v1.0.0 question: read the open issues and PRs, check
the validator's own falsifiability rule by rule, fix the real defects, and name
what is blocked. Every guard added or repaired was broken on purpose, watched
fail, restored, and watched pass; both directions are in the commit messages.
Three things to read first
1. The v1.0.0 readiness assessment is in
docs/plans/v1.0.0-readiness.mdVerdict: not ready, and nothing remaining is engineering in this repository.
There was no single v1.0.0 checklist but four overlapping ones that disagree
about the world, so the assessment merges them and marks where a document is
wrong about its own repo. Three items are left:
v0.10.0disqualifieditself in its own release note: it added
TODS-E207and changedcoverage-manifest behavior in three commands.
## Unreleasedchangesbehavior again, so the earliest qualifying release is two away. Maintainer.
state=open,merged=false, last updated 2026-07-17. FreezingTODS-E204vs
TODS-W408today freezes this repo's reading of an issue thread ratherthan published text. Upstream, and the only blocker with no date attached.
gh api repos/ChelseaKR/tods-validate/environmentslists exactly one environment,github-pages. There is nothing to scope a trusted publisher to.2. Three documents are wrong about the branch ruleset
Read live on 2026-08-28: ruleset
protect-main(id 18752857) is active onrefs/heads/main.DEFINITION_OF_DONE.md("Not yet enabled as a live GitHubruleset") and
docs/v1-contract-audit.md("no ruleset is enabled on therepository") both say otherwise. And the live one is not the committed
docs/rulesets/main.json:docs/rulesets/main.jsonmainprotect-mainaccessibility,action-self-test,citation,contract,perfTwo consequences:
contractis not a required check. The gate that protects the v1 publiccontract can be red on a pull request that merges. For a v1.0.0 story that is
the one required check that actually matters.
gh api ... /rulesetswith that body creates a second ruleset rather thanupdating the live one, and it replaces the merge requirements wholesale. Its
pull_requestrule setsrequired_approving_review_count: 1withrequire_last_push_approval: trueandrequire_code_owner_review: true; on arepository whose
CODEOWNERSnames one person, that is unsatisfiable for asolo maintainer.
Recommendation: export the live ruleset and diff it before applying
anything. This is a documentation defect, not a request to change a setting.
No live setting was changed by this pass and none should be changed on the
strength of a file that does not describe what is there.
3.
v0is a lightweight tag pointing at the v0.5.0 commitv0resolves to097427f, the v0.5.0 release commit. Anyone following theActions convention and pinning
ChelseaKR/tods-validate@v0today gets softwarefour releases old, including a fixed companion-GTFS fail-open. Deleting a
published ref is a live action and out of scope here; the gaps ledger records
it and the two commands. Worth doing before v1.0.0 either way, because
v1will invite exactly the same pin.
The falsifiability census: 43/43 clean
Enumerated by AST, not regex -- a
[A-Z_]+pattern missesTODS-E207.@rule(...)registrations and 43 distinct literalFinding(rule_id=...)values, sets identical, and zero non-literal
rule_idconstructionsites, so no rule ID can be assembled at runtime and escape the census.
tests/fixtures/invalid/<ID>/fixture; 0 silent.finding disappear -- which is what rules out a finding produced by a
different rule under the same ID.
Nothing in the rule set is decorative, and it was true for a structural
reason: the fixture-per-rule parity assertion is not something a passing
implementation can satisfy by accident. One residual is stated in the
assessment (B7): the additional rule IDs in each fixture's expected set were
seeded from a run and are reviewed by eye; the load-bearing assertion is not.
The defects found and fixed
4647e38gitleaks --source .scans history, not the working tree. A root-level file holding an AWS key pair, a GitHub PAT and a Slack bot token, never staged, gave "283 commits scanned / no leaks found", exit 0. With--no-git: "leaks found: 1", exit 1. The recipe's own comment claimed "working tree + history".101cffbpy.typed. A five-line consumer runningmypy --strictagainst the installed package gotSkipping analyzing "tods_validate": ... missing library stubs or py.typed marker, exit 1. Freezing a semver promise on an API every downstream type checker refuses to look at.897fd0emypy files = ["src"]while ruff coveredsrc tests scripts, leaving the gate scripts themselves the least verified code here. 34 files checked becomes 44; one real error inspec_watch.py, where-> strwas a promise mypy had never checked.9041979run(feed)'s result and divided an assumed row count by CPU time, so a validator that had stopped reading the feed would burn no CPU, report an enormous rate, and pass further inside the budget than a correct one. Every existing test stubsmeasureout, which is why nothing noticed.fa3d92a--log-format json; the flag is nowhere insrc/, and no module importsloggingat all, so there is no stream to format.2dba719TODS-I601's 6h bound was never exercised by its 8h fixture. Moving the constant to seven hours left the conformance test passing, exit 0.8c5c2f3pull_requestjobs (perf, and the path-filtered VS Codepackagejob) had nomakeequivalent and no mention in the header that enumerates CI-only work.3e4d01cread.to_dataframeand__version__are in the v1 contract and were named in none of the 52 test modules. A 90% coverage floor cannot see an export nothing imports.5899608docs/api.mddescribedFindingas 7 fields and 2 helpers; it has 10 and 3, andreport.schema.jsonrequires the three omitted plusfingerprint().docs/read-api.mdhad no currency stamp, somake docs-checkhad nothing to fail on.Verified
13 of 13 gates PASS, 701 tests (from 664), coverage 91.80% against a
90% floor. The AST rule census and falsifiability harness were re-run after
every change and stayed 43/43.
Note for anyone reproducing this:
.venv/binmust be onPATH, which iswhat CI does (
ci.ymlwrites it to$GITHUB_PATH). Without it six gates failfor environment reasons on a laptop. Not a repository defect, but a sharp edge:
CONTRIBUTING.md's local instructions and CI'sPATHstep describe twodifferent setups.
Base and merge notes
This branch is cut from
a019bbe(v0.10.0), which is 5 commits behindmain(7a25056).a019bbeis an ancestor ofmain, so the diff here isexactly these 11 commits. It does not merge cleanly, and the conflicts are
measured rather than guessed --
git merge-tree --write-tree origin/main HEADreports exactly three files:
CHANGELOG.md-- both sides add to## Unreleased. Concatenate.Makefile-- thesecretsrecipe and the CI-only header paragraph, bothof which docs+fix: a multiyear phase plan, and phase 1 (three gates could report a pass they had not earned) #151/phase 2: everything that must be true before v1.0.0 (not the release) #152/phase 3: measure the two axes nobody was measuring, audit the pages nobody was auditing #153 also touched. Take both sets of changes; the header
paragraph is now pinned by
tests/test_ci_gate_parity.py, so a wrongresolution turns that test red rather than passing quietly.
scripts/check_doc_currency.py-- theSTAMPEDlist. Union the entries;tests/test_doc_currency.pynow derives its parametrize fromSTAMPED, so adropped entry is visible.
scripts/spec_watch.pyauto-merges, contrary to the note in the workingplan that expected it to need a hand re-apply.
docs/rulesets/main.json, whicharrived with #152, does not exist in this checkout and nothing here edits it,
deliberately, so no conflicting duplicate is created; the divergence is written
up in the readiness assessment instead.
Rebasing was left to the maintainer rather than done here, because every
measurement in this PR (701 tests, 91.80%, 43/43 census) was taken against
a019bbeand a rebase would invalidate the evidence without re-running it.Every candidate finding was re-checked against
git show origin/main:<path>before being called a defect. Two fail-opens found during the audit turned
out to be already fixed on
mainby #151 (spec_watch.pytreating anunrecognised document as "in sync";
check_npm_audit.pydisarming its owncross-check on an unparseable report) and are not re-fixed here.
PRs #154 and #155 (stacked) and draft #79 were read and deliberately not
touched; #154/#155 already report
CONFLICTINGagainstmain, and nothinghere edits a file they add.
Left undone, with the reason
--log-format json. Addingstructlogas a second runtime dependency toa tool that deliberately has one, to format log records that do not exist, is
a product decision. Recorded as a gap with both options written out.
tests/under mypy.scripts/was the half that gates merges.citing. Add a structure warning rule (TODS-x1xx) for a recognized-but-unexpected file #143 and Add a second advisory rule (TODS-x6xx, opt-in) #144 both need a citation chosen and defended, and inventing
one to close an issue would be the exact failure this validator exists to
refuse.
Two issue texts did not survive checking: #145 is already fixed (landed in
#152; close it against that PR), and #146's premise predates #150, which
now boots the live page in a real browser -- what remains is a dated human
record, narrower than the issue says.
🤖 Generated with Claude Code