docs: correct the README claims the Python build made wrong - #5
Conversation
Audited every checkable claim in README.md against the code on main, with the commands run rather than read. Five were wrong or incomplete: - "every script here uses --env-file" — `usage` does not, and never did; it reads a CSV and calls nothing. Narrowed to scripts that need a key. - "Run those from the repo root ... where both languages expect to find usage.csv" — the Python side deliberately does NOT depend on the working directory. pyweather/usage.py resolves the ledger from __file__ and __init__.py finds .env by walking up, precisely so `uv run` lands on the same ledger from anywhere. `uv run weather` from inside pyweather/ works. - the command-parity gate is described as having one exception; it has two kinds (the assistant:streaming rename, and the three npm-only gates), and it also checks that each entry point resolves to a real `def main`. - `typecheck` was missing from the list of npm scripts with no Python counterpart, and pyweather/'s own helpers went unmentioned. - the cost table omitted `agent`, `injection`, and `verify:docs`, and nothing said which commands need WEATHER_API_KEY as well. Gates: typecheck, typecheck:py, verify:docs all exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The merge added an explicit request timeout to both src/weather.ts and pyweather/weather.py — the one substantive thing Part 7 gained in both languages. verify:docs kept the listings in sync, but it cannot see that the prose around them stopped describing the file. - docs/typescript.md's "Read what you just wrote" still promised "four ideas in that file" and enumerated the pre-merge four, skipping the most transferable one. fetch having NO default timeout is exactly the kind of thing a beginner assumes is handled for them. - docs/python.md's Part 7 comparison table had no row for it, though the defaults genuinely differ in kind: fetch has none, httpx silently has 5s. That divergence is what the table exists to make visible. Prose only — no code block touched. typecheck, typecheck:py, verify:docs all exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Follow-up: extended the audit past
Prose only — no code block touched, so the doc/code coupling gates are unaffected. All three gates re-run after the edit: Two findings left deliberately unfixed, for a human to decide:
Everything else in |
| The four helpers have counterparts too — `pyweather/text.py`, `config.py`, | ||
| `usage.py`, `weather.py` — plus one with no TypeScript equivalent: | ||
| `pyweather/__init__.py`, which loads `.env` once for the whole package where | ||
| the npm scripts each pass `--env-file`. |
There was a problem hiding this comment.
[nit] The new helpers sentence says pyweather/__init__.py loads .env once “where the npm scripts each pass --env-file.” That reintroduces the overclaim this PR just corrected on line 74. In package.json, only the twelve lesson scripts that need a key pass --env-file; usage, typecheck, typecheck:py, and verify:docs do not. pyweather/__init__.py already names the usage exception (“Thirteen scripts, twelve copies of --env-file=.env”).
Suggestion: Reuse the wording from line 74: “where the scripts that need a key pass --env-file.
PR 5 promoted "fetch has no timeout at all" to a Part 7 teaching point. It is false. undici, the HTTP engine behind Node's fetch, defaults headersTimeout and bodyTimeout to 300 seconds. Measured on Node v24.14.1 against a server that accepts the connection and never responds, fetch rejects after 301.0s with UND_ERR_HEADERS_TIMEOUT. The claim lived in six places, not the two a first read found: docs/typescript.md the Part 7 prose, and the src/weather.ts listing docs/python.md the comparison-table row, and the weather.py listing src/weather.ts the comment above the fetch call pyweather/weather.py the comment above the httpx.get call All six now say the same thing: both runtimes ship a default, they differ by 60x (300s against 5s), and they differ in kind as well as size — AbortSignal.timeout is one deadline for the whole call, where httpx applies its timeout to each operation separately. That divergence is a better Part 7 lesson than the absolute it replaces, and it is exactly what the Idea | TypeScript | Python table exists to surface. Also in this pass: - docs/python.md still carried the "run every command from the repository root" instruction this PR deleted from README.md, 380 lines before the LEDGER comment that disproves it. It was the highest-traffic copy. - The comparison table gained the follow_redirects row that the code comment names and the table skipped. - src/weather.ts and pyweather/weather.py headers said "four ideas" where the document now says five. verify:docs strips comments, so no gate sees this. - README: models moved out of the paid bucket (models.list() spends no tokens and logs no row), the --env-file overclaim removed where it had been reintroduced 150 lines after being fixed, the typecheck "not mirrored" sentence narrowed to what is true, uv's directory independence bounded to inside the repo, and the Part 7 summary row updated to match the lesson. Gates: npm run typecheck 0, npm run typecheck:py 0, npm run verify:docs 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsAeobZPagwRXXXDzqE6bW
README promised that `verify:docs` catches a `src/` file drifting from its listing in `docs/typescript.md`, and named the exact line it would report. That is true of code and false of comments: `stripTypeScript` and `stripPython` remove comments from BOTH sides before the diff gate compares them, so a teaching comment can drift out of step with its listing — or say something flatly untrue — with all six gates green. This is not incidental. It is how the timeout claim corrected in the previous commit stayed wrong in six places through a passing CI, and in this repo the comments ARE the teaching. The promise now carries its exception. Also: "Every call is logged to usage.csv" had the same defect as the cost table above it — `models` calls the API and logs no row. Narrowed to calls that spend tokens, matching the fix at line 134. Gates: npm run typecheck 0, npm run typecheck:py 0, npm run verify:docs 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsAeobZPagwRXXXDzqE6bW
|
Review pass over this PR, and the fixes for it. Two commits pushed here ( The one that matteredThis PR promoted " That matters beyond the wording, because the second commit added the The claim lived in six places, not the two a first read finds:
Also corrected here
The learning
Second learning, cheaper: this PR's premise — "the one document with claims that no gate covers is GatesBoth commits, exit codes checked individually, in a clean worktree:
No Claude API call was made. Left undone, deliberately
🤖 Generated with Claude Code |
Mission-end documentation gate for kickoff issue #1 (merged as #2,
309f021..a381064).The mission's deliverable was itself mostly documentation, and
docs/python.md,CLAUDE.md, and the four cross-document navigation headers all came out of it accurate. The one document with claims that no gate covers isREADME.md— so it got the audit, against the merged code and by running the commands rather than reading about them.What changed, and what it was authored from
--env-filearrived, and every script here uses it"package.jsongivesusageno--env-file— it reads a CSV and calls nothing.pyweather/__init__.py's own docstring already said so: "Thirteen scripts, twelve copies of--env-file=.env—usageis the one exception."pyweather/— that is whereusage.csvlives, and where both languages expect to find it"pyweather/usage.pysetsLEDGER = Path(__file__).resolve().parent.parent / "usage.csv"specifically becauseuv runmakes no promise about the working directory;pyweather/__init__.pyusesfind_dotenv()'s upward walk for the same reason. Verified:uv run weatherfrom insidepyweather/succeeds and reads the root.env, andLEDGERstill resolves to the repo root.scripts/check-docs.tshas two kinds —RENAMED_ENTRY_POINT(assistant:streaming→assistant-streaming) andNPM_ONLY(typecheck,typecheck:py,verify:docs) — and a third check the README didn't mention: every entry point must resolve to a module that defines thedef mainit names.typecheck:py… andverify:docsaren't mirrored"typecheckisn't mirrored either. Also added the Python helpers:pyweather/has counterparts to all foursrc/helpers, plus__init__.py, which has none.agent,injection, andverify:docsentirely, and nothing anywhere said which commands needWEATHER_API_KEY. The five weather-tool scripts are exactly those importingweather.js/.weather.Everything else in the README checked out and was left alone: the six gates (the count matches
verify:docs's own output — structure, command parity, then compile/ordering/diff/coverage per document), the fifteenusage.csvcolumns and their five groupings, the four-term cost formula,by model / by session / by cachingin both usage reporters, the three model IDs insrc/usage.tsandsrc/bench.ts, the status table, and the two setup documents' links.Gates
Run in a clean worktree off
origin/main, exit codes checked individually:npm run typecheck→ 0npm run typecheck:py→ 0npm run verify:docs→ 0 (structure: 7 Markdown files,command parity: 16 npm scripts and 13 [project.scripts] entries agree, both documents clean)Also run, to check claims rather than to gate:
uv sync --locked,npm run weather,uv run weather(from the root and from insidepyweather/),npm run usageanduv run usage(both with no ledger, and both over one synthetic ledger — byte-identical reports from the same file). No Claude API call was made.🤖 Generated with Claude Code