[juaristi22]

build_bundle held every source package's consumer rows until the end of the build. It kept them in one list, grouped all of them by aggregate and semantic fact key, and only then wrote and counted them. Held rows cost about 7.4 KiB each (276,703 rows took 2.0 GiB to load), so the default bundle's 1.74M rows took about 12 GiB just to hold. On CI, main's bundle step peaked at 15.0 GiB at this PR's base, close to the runner's memory. #410 adds about a fifth more facts, and its bundle job was shut down twice. #410 is stacked on this PR.

What changes

• Each package's rows are appended to consumer_facts.jsonl.partial and added to a running coverage count as soon as they're read. The rows and the package's suite report are then freed before the next package builds.
• The file is renamed to consumer_facts.jsonl once every package is in, so an interrupted build never leaves a partial facts file under the final name.
• _BundleCoverage keeps the coverage counts and each identity's distinct values. For each aggregate and semantic fact key, it records where the first row came from: its source table and legacy key. Only keys seen again collect more, so duplicate reports need no rows.
• build_bundle_coverage(rows) keeps its signature and output, and uses the same counter.
• The metadata pool is now per package. A bundle-wide pool would keep every package's metadata alive.

Memory

Figures are peak resident memory (max RSS) in GiB.

• CI, the whole default bundle (1.74M facts). This branch's bundle step peaked at 3.4 GiB, against 15.0 GiB on main's run at the base, 6475380. GNU time reported 3,583,520 and 15,728,820 KiB.
• CI, UK land value inputs for chronicle#408: MHCLG land values, VOA's 2026 stock, EPC D3, council tax by authority, the Market Value Survey, land by sector and HMRC stamp tables #410 stacked on this branch (2.06M facts). Its bundle step passed and peaked at 3.4 GiB (3,581,556 KiB). Before the stacking, its bundle job was shut down twice.
• The UK suite on one Mac (build-bundle --suite uk, 1,585,295 facts, 309 packages). It peaked at 4.0 GiB, against 12.8 GiB for main 6475380, built with the same command on the same night. Python's own peak footprint was 3.7 GiB.
• The peak is now the largest package's build and rows, plus the running counts. The counts are mostly one entry per distinct fact key.
• Time.
  • On the Mac, the UK suite took 39 min against 45, with slightly less CPU time.
  • CI's bundle step took 2h51m against 2h29m at the base. That runner was also about a fifth slower on tests this PR doesn't touch: the UK calibration additions took 207 s against 174 s.

What stays the same

• Outputs. The UK suite's four outputs are byte-identical to main 6475380's.
  • consumer_facts.jsonl (7,993,174,294 bytes) matches the facts file of main's UK consumer artifact, sha256 bcb226fd…. build-consumer-artifact writes that file from the bundle's.
  • coverage.json, source_packages.json and reports/build_bundle.json match main's. Main's bundle folder was deleted before the comparison, so I rebuilt these three from its logged build report with the bundle's own writer. The rebuilds match the original files' sizes.
  • A nine-package subset (US, UK and NZ, 276,703 facts) is byte-identical too.
• Pinned counts. CI's whole-bundle test passes with main's pinned counts, including the 2,324 semantic duplicate keys.
• Rows without a fact key. A row without an aggregate or semantic fact key still raises KeyError when duplicates are computed, as it did when every row was grouped by its key.

Tests

• test_build_bundle_frees_each_packages_rows_before_the_next_builds builds two packages that share a fact. It checks three things:

  • the first package's rows are freed before the second builds;
  • the facts file and the coverage match counting all the rows at once;
  • the shared fact is still reported as a duplicate.

  Run against main's build_bundle, it fails: both of the first package's rows are still held when the second builds.

• CI's bundle job: 33 passed, the whole-bundle test included.

• Locally, tests/test_chronicle_bundle.py without the whole-bundle test: 32 passed. The 19 package test files that call build_bundle_coverage: 3,668 passed.

🤖 Generated with Claude Code

[vahid-ahmadi]

Review of 511b857. I read the diff against the base (6475380, current main) side by side with the old build_bundle, build_bundle_coverage and _duplicate_key_reports, and checked CI. I couldn't run the tests locally: the environment blocked installing the package. So this rests on reading the code plus CI's bundle job, and I didn't run the bundle build either.

Should

1. tests/test_chronicle_bundle.py:3318: report.coverage == build_bundle_coverage(rows) compares the new counter with itself, because build_bundle_coverage now wraps _BundleCoverage. So the new test proves the rows are freed, not that the output matches the old path. The equivalence evidence is your UK byte comparison plus CI's pinned counts, which check counts only, not duplicate content. The one part with new logic is _FactKeyIndex, which records the first row's origin and adds the rest only when a key repeats. Suggest pinning its output directly: three rows sharing one aggregate key, across two source tables, with two different legacy keys and one row with no legacy key. Assert the full duplicates entry, i.e. count 3, both sources sorted, both legacy keys. That locks in the first-row capture, which the current tests only cover for the two-row cross-epoch case.

Nit

2. chronicle/bundle.py:826 / :912: if something raises mid-loop (for example _load_jsonl's epoch validation) or in duplicates() (the missing-key KeyError), consumer_facts.jsonl.partial stays on disk. For the UK suite that's up to about 8 GB. The old path left no facts file at all. Since a rerun needs replace=True anyway, this only costs disk space. A try/except BaseException: partial_facts_path.unlink(missing_ok=True); raise around the loop and rename would tidy it.
3. chronicle/bundle.py:820-821, tests/test_chronicle_bundle.py:3264, changelog.d/bundle-streams-consumer-rows.fixed.md:3-4: these say "1.66M rows ... 15.4 GB". The PR body says 1.74M facts and 15.0 GiB (15,728,820 KiB, about 16.1 GB). Worth using one set of figures and one unit.

Checked and fine

• Row order and bytes: packages are still handled in build_sources order, rows are appended in load order, and json.dumps(sort_keys=True) is unchanged. There was no cross-package dedup before and there is none now.
• Duplicate reports match the old grouping. Count, sorted sources and canonicalised legacy keys are the same; canonicalising after the set gives the same set as before. Falsy legacy keys are still dropped (or None vs the walrus test), and sorting still uses _identity_sort_key.
• unique_counts: len(index.first) equals the old _unique_count, which also skipped rows without the key. The five identity sets are unchanged.
• A missing fact key still raises KeyError. It just happens in duplicates() instead of during grouping. That's still before the rename, so the final facts file is never written.
• The cross-package label and geography checks already built up per package. They are untouched and still get every package's rows.
• The pinned whole-bundle counts (semantic_duplicate_key_count: 2324 etc.) are unchanged in the test file and pass in CI.
• Memory: no row list survives the loop. There is no continue between the load and del rows, suite_report, and BundleSourceReport copies only counts and outputs. What remains grows with distinct keys, not rows. The per-package metadata pool is right. sorted() only runs on counts and the repeated keys.
• The removed helpers (_duplicate_key_reports, _counts_by, _unique_count, bundle _write_jsonl) have no other callers. suite.py has its own _write_jsonl.

CI at 511b857: all green. The bundle job passed in 2h54m56s and the surface job in 1h42m. The last four bundle runs on main took 1h51m to 2h55m (2h33m at the base 6475380), so the 2h55m is within main's spread. It isn't evidence that streaming slows the build.

The fix looks right, and I found nothing that weakens a cross-package check. Item 1 is the only gap I'd close before this leaves draft.

🤖 Generated with Claude Code

[juaristi22]

Thanks, Vahid. All three are addressed in b71d03d, pushed together with 36835ab.

1. Duplicate report test. test_bundle_coverage_duplicates_keep_each_keys_first_row pins the whole duplicates value for three rows that share an aggregate key: count 3, both source tables sorted, both legacy keys. The first row is the only one from Table 1.2 and the only one with its legacy key, so losing the first row's origin fails the test. Another row has no legacy key. A semantic key whose first row has no legacy key is pinned too. For these rows, main's build_bundle_coverage (6475380) returns the same coverage as this branch, so the pinned values are the old path's.
2. Partial file. build_bundle now removes consumer_facts.jsonl.partial if anything raises before the rename, then re-raises. test_build_bundle_removes_its_partial_facts_file_when_it_fails writes the first package's rows and then fails to load the second's. It fails on 511b857 and passes now.
3. Figures. The comment in build_bundle, the test docstring and the changelog now give 15.0 GiB for the default bundle's 1.74M rows, as the description does. 36835ab also updates the note above the CI bundle step.

Locally, tests/test_chronicle_bundle.py without the whole-bundle test: 34 passed. CI is running on b71d03d.

[vahid-ahmadi]

Round 2 at b71d03d. I read the diff from 511b857 (36835ab and b71d03d) against _FactKeyIndex and the fixture rows. The environment blocked running the tests locally again, so this rests on reading the code plus CI, and I didn't run the bundle build.

1. Duplicate report test: fixed. test_bundle_coverage_duplicates_keep_each_keys_first_row pins the whole duplicates value. The aggregate key's first row is the only one from Table 1.2 and the only one with the second legacy key, so dropping the first row's origin in _FactKeyIndex.add would fail it. The semantic case covers the other branch too: its first row is the one with no legacy key, which is the set() if first[1] is None path. I hand-checked the expected values against the fixture: the legacy keys sort 079f… before 7f8e…, and count 3 / 2 and the two unique_counts follow from the edits.
2. Partial file: fixed. The loop, the duplicate checks and the rename are now in one try, and except BaseException removes the partial file and re-raises, so Ctrl-C is covered too. The rename is the last statement in the try, so a successful build never deletes it. Package build failures are still caught inside the loop and recorded as errors as before. Only exceptions that escape the loop reach the cleanup, such as _load_jsonl's epoch check, which is what the new test triggers. If writing coverage.json or the report fails after the rename, the final facts file stays, as it did on main.
3. Figures: fixed. The code comment, test docstring, changelog and CI note now all say 15.0 GiB for 1.74M rows, matching the description. The new test's "about 8 GB" for the UK facts file matches the 7,993,174,294 bytes in the description.

No new findings. Moving the shared stub into _stub_source_suites leaves the frees-rows test's assertions unchanged.

CI at b71d03d: still pending. Both the bundle and surface jobs started at 12:07 UTC, and the bundle job usually takes about 3h. So the two new tests and the pinned whole-bundle counts haven't yet passed at this head.

Nothing more from me once CI is green.

🤖 Generated with Claude Code