fix(example): give the source cache an age, and a way to discard it - #29
Merged
Conversation
`data/source/` is a cache that never expired. Reuse was keyed on the files being present and internally coherent -- counts agreeing with recorded metadata, ownership and active-status predicates holding -- and the log called that "completeness-verified" and "semantics-verified". Both are true statements about the bytes on disk and neither says the data is current. The two are indistinguishable in a log until someone regenerates the committed artifacts from a stale cache. The result is self-consistent, passes validate and verify, and cannot be reproduced by CI, which always starts cold and fetches what the services publish today. The failure then surfaces as the project.qgz currency check, a long way from its cause. That happened on 2026-09-09: a regeneration silently reused two-week-old sources and reported 518 candidates against the current 517. So: - `--refresh` discards the cache and re-fetches every service. Run it before regenerating the committed artifacts; - reuse now logs the cache's age, and reuse beyond CACHE_MAX_AGE_DAYS (7) warns that coherent is not the same as current; - the log wording no longer implies currency. The example is regenerated in the same commit, because the run record hashes pipeline.py and tests/test_verify.py holds it to that. The refreshed sources produce a byte-identical project.qgz, which is independent evidence that this environment matches CI's. Also corrects two manifest prose figures that contradicted the derived `row_count: 79082` beside them, and the same count in the README. Those were stale before this change; the refresh is what exposed them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5178f4d35f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+145
to
+148
| if refresh: | ||
| if path.exists(): | ||
| log.info("--refresh: discarding cached %s", label) | ||
| return False |
There was a problem hiding this comment.
When data/source/ contains artifacts from an older pipeline version, --refresh only bypasses reuse and overwrites the currently known files; it never discards the other cached files. finalize_run() inventories every file under SOURCE, so these leftovers contaminate the supposedly cold-refresh input list and hash—the newly committed run already includes unused parcels.geojson, roads.geojson, parcels.json, and roads.json, none of which the current pipeline reads and none of which a cold CI run creates. Clear or explicitly prune the cache before refetching so --refresh produces the same attestation as a cold run.
Useful? React with 👍 / 👎.
Three findings from review, all real. **The refresh was not cold.** It bypassed reuse file by file, so anything an older pipeline version left in data/source/ survived it. The run record inventories every file under that directory, so four stale artifacts -- parcels.geojson, parcels.json, roads.geojson, roads.json, none of which this pipeline reads and none of which a cold checkout has -- entered the committed inputs_hash. The attestation the flag exists to produce was therefore not the one CI produces: 12 inputs against a cold run's 9. --refresh now empties the directory before refetching. **A damaged cache could block the refresh meant to replace it.** The cached files were parsed to test coherence before the refresh check ran, so truncated JSON raised before the discard. Emptying the directory first removes the parse entirely. **The cadastre recorded a snapshot it was not.** Its download timestamp and version were frozen literals (2026-08-25), its ETag and size_bytes were prose nothing updated. A daily snapshot behind a stable URL is only identifiable from the response that delivered it, so a refreshed download filed new bytes under an older date -- exactly the provenance guarantee this example exists to demonstrate. The download now captures ETag and Last-Modified, keeps them in a sidecar beside the file, and finalize_run writes identity and size back from what was measured. The correction is visible in the regenerated manifest: the ETag moved to 5ca41422..., and Last-Modified dates the archive to 2026-09-09, not the 2026-08-25 the manifest had been claiming through several runs. size_bytes now matches the file at 55762944; it had been 55746560. A cache is valid only with its sidecar, so a legacy cache refetches once. The analysis is unchanged: 517 candidates, 66/84/367 tiers, and a byte-identical project.qgz. Also drops the run record from the first commit of this branch, whose inventory carries the four stale files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg
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.
The bug
data/source/is a cache that never expired. Reuse was keyed on the files being present and internally coherent — counts agreeing with their recorded metadata, ownership and active-status predicates holding — and the log called thatcompleteness-verifiedandsemantics-verified. Both are true statements about the bytes on disk. Neither says the data is current.Those two look identical in a log until someone regenerates the committed artifacts from a stale cache. The result is self-consistent, passes
validateandverify, and cannot be reproduced by CI, which always starts cold. The failure then surfaces as theproject.qgzcurrency check, a long way from its cause.That is what happened on 2026-09-09: a regeneration silently reused two-week-old sources and reported 518 candidates against the current 517.
The fix
pipeline.py --refreshdiscards the cache and re-fetches every service. Run it before regenerating the committed artifacts.CACHE_MAX_AGE_DAYS(7) warns that coherent is not the same as current.The README and the maintainer debugging note both point at
--refreshat the moment someone would need it.Why the example is regenerated in the same commit
The run record hashes
pipeline.py, andtests/test_verify.py::test_latest_run_attests_the_committed_pipelineholds it to that — editing the pipeline without re-running is a failing test by design.I regenerated with
--refreshin an environment matching the manifest pins (Python 3.12, duckdb 1.5.5, pyproj 3.7.2). The refreshed sources produce a byte-identicalproject.qgz, which is independent evidence that this environment matches CI's and that #28's regeneration is still current.One correction rolled in
The manifest's prose said the cadastre holds 79,056 parcels while the machine-derived
row_count: 79082sat a few lines above it in the same file, and the README repeated the stale figure. Both are now derived from the same number. That contradiction predates this change; the refresh is what exposed it.Verification
validateandverifyboth reportwarning, this project's honest status--refreshconfirmed to discard and re-fetch all three sources; the cache helper's six branches (fresh, stale, incoherent, refreshed, absent, arg parsing) exercised directly🤖 Generated with Claude Code
https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg