Skip to content

Add build_basic_tomes: the header and a tome per channel in one walk - #40

Merged
billfreeman44 merged 4 commits into
masterfrom
worktree-dsdk-review-builder
Sep 29, 2026
Merged

billfreeman44 merged 4 commits into
masterfrom
worktree-dsdk-review-builder

Conversation

@billfreeman44

@billfreeman44 billfreeman44 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

What it builds

TomeCuratorFs.build_basic_tomes(channels) builds a data scientist's standard tome set in one walk over the matches: the header tome, and one tome per channel whose rows are tagged match_key. Those are the tomes create_header_tome, a has_<channel> subheader and a make_tome per channel build today. The builder visits each match once and reads its manifest, its header row and every requested channel with pyarrow, 4 matches at a time. Today's script reads each manifest at least 23 times.

  • A channel's tome holds the matches that have the channel, in header order. Its header copy holds their header rows, like today's has_<channel> subheader. A match whose file is empty is in the keyset with no rows, as with make_tome. A channel that no match has gets no tome.
  • Only the header and the channel tomes are written. all, map_* and platform_* subheaders only filter the header, so the caller still makes them with create_subheader_tome, and the builder stays focused.
  • Pages are zstd by default and carry pandas metadata, so TomeLoader reads back the values and dtypes a make_tome tome gives.
  • The make_tome iterate/concat API is unchanged. TomeWriterFs.write_page now also takes a pyarrow.Table. loader.py and reader_fs.py are untouched.

API

built = curator.build_basic_tomes(
    channels,                            # names, or {"channel": ..., "columns": [...]}
    tome_name="basic_{channel}.{dates}", # {dates}: the header's first and last match_date
    header_tome_name=None,               # the curator's default header
    keys=None,                           # every match in the collection, or these manifest keys
    max_page_size_mb=256,                # Arrow bytes; None writes one page per tome
    compression="zstd",
    read_threads=4,                      # 0 or 1 reads inline
    behavior_if_complete="pass",         # "overwrite" rebuilds, "fail" raises
    behavior_if_partial="overwrite",     # "pass" leaves it, "fail" raises
)
built.header                   # TomeLoader
built.tomes["player_death"]    # TomeLoader of basic_player_death.<dates>
built.dates                    # "2023-11-10,2026-07-10"

build_basic_tomes_from_fs(...) next to create_header_tome_from_fs does the same without a curator. The full reference is in the docstring and in a new section of docs/tome-data-model.md.

Names and staging. {dates} is known only once every header row has been read, so when the builder builds the header it writes the channel tomes to tome/<ds_type>/.building/<header>/<channel>. At the end it moves each one to its name, which is a rename on the same disk. Before it moves any page file, it marks the destination partial. It writes the tome's header copy before marking the manifest complete.

Resuming. A complete tome is kept, so a rerun whose tomes are all complete reads no match. An interrupted build leaves no partial channel tome, only the staging folder and a partial header, and the next call starts over. By default a partial tome, such as one an interrupted make_tome left, is built again. There is no continue, because the build is one walk: continuing one tome would mean reading every match again anyway.

How the dtypes match make_tome

make_tome reads each match with pandas, joins a page with pd.concat, and writes it with pandas. The builder instead joins each page's Arrow tables with pa.concat_tables(promote_options="permissive"):

  • int64 with double becomes double;
  • null-typed columns take the other files' type;
  • missing columns are filled with nulls.

The page's pandas metadata names the dtype pd.concat would give the same matches. The builder gets it by running pd.concat on two-row stand-ins, one per distinct file schema and null pattern, which carry each file's type and pandas metadata. That is how Int64 wins over the int64 and object some csds versions declare for the id columns: the per-file pandas metadata is merged, not taken from the first file. The builder then checks that the joined page reads back with exactly those dtypes. If it doesn't, or pyarrow can't join the tables (differing categories, for example), that page is built with pd.concat as make_tome would build it, and the builder logs "Page built with pandas". No real-data page needed that.

The stored pandas index (__index_level_0__, or a named index) is dropped, as make_tome's ignore_index concat drops it. Tables are merged every 256 matches, so a page of many small matches doesn't hold tens of thousands of tables. Page cuts count Arrow bytes (Table.nbytes), which count strings by their bytes where make_tome counts pandas' deep memory, so a page holds more rows for the same max_page_size_mb.

Measured

Setup. Every run was cold, on 200 matches never read before by this work or by the P1 benchmark. The matches were read from the USB drive and the tomes written to the NVMe. Each run built the 21 standard channels with 256 MB pages, in its own process, with no other busy python. The baseline is P1's one-pass pyarrow prototype, timed in the same session. It reads the manifest and the 21 channels but not the header channel.

Seconds per match:

build inline (read_threads=0) 4 threads
this builder, one walk 0.385, 0.385 0.261, 0.263
P1 prototype, one pass 0.405, 0.404, 0.410, 0.412 0.256, 0.254, 0.272, 0.265
a two-step draft of this builder 0.493, 0.495 0.354, 0.349
  • Against the prototype. The builder matches it, although it also reads each match's header channel and writes the header tome: 23 files per match against the prototype's 22. 4 threads are 1.47x faster than inline.
  • A 600-match set with 4 threads ran at 0.241 s per match.
  • Warm, with the files cached, 200 matches and 4 threads took 4.4 s. make_tome's per-channel loop took 20.4 s on the same files.
  • The whole standard set (46,637 matches) projects to about 3.4 h from the USB drive with 4 threads, or 5.0 h inline. July's build took 28.3 h.
  • Why one walk. The first draft scanned every match's manifest and header first, so that {dates} names were known before any channel page was written, and then read the channels. Visiting every match folder twice cost 20 to 30% on the USB drive. So the builder now visits each match once, and stages the channel tomes until the dates are known.

Equivalence on real data

These checks read the CSDS collection on D: read-only and wrote output to C:\polars_bench\p4. The builder's tomes were compared with tomes that make_tome built on dsdk master 1ab01d1 (zstd), following make_standard_tomes.py: a header, a has_<channel> subheader per channel, and make_tome with match_key. Both were loaded through TomeLoader. The sample was 200 random matches of the July header (seed 20260928), across the 21 standard channels. player_connect, player_fall and player_interaction were left out because no manifest has them.

run pandas pages per tome (builder / make_tome) result
8 MB pages, 4 threads 3.0.6 1 to 5 / 1 to 4 21 of 21 tomes and the header equal
1 MB pages, 4 threads 3.0.6 1 to 32 / 1 to 20 21 of 21 equal
1 MB pages, inline 2.3.3 (both sides) 1 to 32 / 1 to 20 21 of 21 equal
600 other matches, 256 MB pages, 4 threads 3.0.6 1 / 1 21 of 21 equal
  • In the 600-match run each page holds all 600 matches, so the builder merged its tables twice per page, at 256 and 512 matches.
  • The 1 MB pandas 3 run also loads equal through dsdk#39's pyarrow loader, at its head fc24eb5.

"Equal" covers:

  • rows and the keyset, in order;
  • the match_key sequence;
  • the column order;
  • every value (assert_frame_equal);
  • every dtype;
  • the header copy against make_tome's copy of the has_<channel> subheader.

Every page was built by pyarrow; none needed the pandas fallback. The tomes had 0 to 107,472 rows. bot_takeover, item_remove and player_name are empty in every sampled match.

De-duplication. The July header lists 10 matches twice. A 30-match set in the header's own order, listing 3 of those matches twice (33 entries), gave a logged "Duplicate match keys dropped" with count 3. All 21 tomes equal make_tome over the 30 matches once each.

Tests

builder_test.py has 39 tests, and writer_fs_test.py gains 2. With #39 merged in, the suite has 225 tests. They pass on the lock's pandas 3.0.6 and on pandas 2.3.3, as in the test-pandas-2 job. Without polars, 186 pass and #39's 39 polars tests skip. pylint gives 10.00, and black is clean.

  • Equal to make_tome on the in-repo fixtures: 28 channels (all but tick, player_status and player_vector), inline and with 4 threads, with small pages. Compared: rows, keyset order, match_key, values, dtypes, the header against create_header_tome, and the header copies.

  • Schema drift on a synthetic collection written by pandas:

    • int64 against double;
    • the id declared Int64, int64 or object;
    • a column added and one dropped;
    • a null-typed empty file;
    • a stored __index_level_0__;
    • an all-null Int64 column;
    • a match without the channel.

    Each case runs with one page and with a page per match, and with tables merged every 256, 2 or 1 matches.

  • The pandas fallback: differing categories match make_tome, merged or not, and are logged.

  • Duplicate keys in keys= and in a kept header.

  • The page split: the page keysets equal a simulation of the Arrow-byte rule.

  • Threads 0 against 4 give identical pages.

  • Resuming:

    • complete tomes are kept without reading a match (the collection is deleted first);
    • overwrite rebuilds;
    • fail raises;
    • an interrupted build leaves no partial tome and the rerun equals a clean build;
    • an interrupted publish over a complete tome leaves it partial, not complete over half-replaced pages;
    • a partial header is rebuilt;
    • a partial make_tome tome is rebuilt, or left with pass;
    • a kept header must hold the given keys.
  • Arguments: bad arguments fail before anything is read, {dates} needs a match_date, and a channel's tome may not take the header's name.

Switching make_standard_tomes.py

That script isn't changed here. Its header block, scan_channel_presence and the per-channel make_tome loop come down to:

curator = create_curator(log=quiet_logger())
built = curator.build_basic_tomes(
    list(dict.fromkeys(args.channels)),
    max_page_size_mb=args.max_page_size_mb,
    behavior_if_complete="overwrite" if args.rebuild_header else "pass",
)
header_df = built.header.get_dataframe()
dates = built.dates
# all.{dates}, map_* and platform_* subheaders: unchanged, from header_df.
  • has_<channel> subheaders are no longer written. Each basic tome's .header is that subheader, and nothing else in the repo reads them.
  • The three channels no manifest has are logged and left out of built.tomes.
  • The builder writes its output to the curator's tome root. That should be a different disk from the CSDS collection.
  • It needs the release that ships this PR.

The CHANGELOG entry is under a new ## Unreleased section, because 3.3.0 is tagged. This PR has no version bump, tag or release. master with dsdk#39 is merged in, and the suite passes on the lock's pandas, on pandas 2.3.3, and without polars.

🤖 Generated with Claude Code

billfreeman44 and others added 2 commits September 28, 2026 18:37
TomeCuratorFs.build_basic_tomes(channels) (and build_basic_tomes_from_fs)
builds the header tome and one tome per channel, each match's rows tagged
match_key: what create_header_tome, a has_<channel> subheader and a
make_tome per channel build, but each match is visited once, for its
manifest, its header row and every channel, read with pyarrow by 4 threads.

- Channel tomes are staged under tome/<ds_type>/.building and moved to
  their names (basic_{channel}.{dates} by default) at the end, once the
  header's dates are known, each with its header copy: the header rows of
  the matches that have the channel.
- Pages are joined with pa.concat_tables(promote_options="permissive") and
  carry pandas metadata naming the dtypes pd.concat gives the same frames,
  worked out from two-row stand-ins per distinct file schema and checked on
  the joined page; a page that doesn't match is built with pandas.
- Pages are cut on Arrow bytes; zstd by default.
- A key listed twice is built once, and the count is logged.
- Complete tomes are kept; an interrupted build leaves no partial channel
  tome and starts over.
- TomeWriterFs.write_page takes a pyarrow.Table too.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The drift fixture's stored index was a range, which pandas keeps as
metadata, so no __index_level_0__ column reached the builder. It is now a
stepped index, stored as a column, and the test checks the file has it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Publishing can corrupt an overwritten tome after interruption, and channel names can collide with the header tome.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds a one-pass builder for header and per-channel tomes while preserving existing make_tome behavior.

Changes:

  • Adds threaded Arrow-based tome construction, staging, paging, and resume behavior.
  • Allows TomeWriterFs to write Arrow tables.
  • Adds comprehensive tests and documentation.
File Description
pureskillgg_dsdk/​tome/​builder.py Implements one-pass tome building.
pureskillgg_dsdk/​tome/​builder_test.py Tests equivalence, paging, threading, and recovery.
pureskillgg_dsdk/​tome/​curator.py Exposes the new curator API.
pureskillgg_dsdk/​tome/​writer_fs.py Adds Arrow table writing.
pureskillgg_dsdk/​tome/​writer_fs_test.py Tests Arrow page serialization.
docs/​tome-data-model.md Documents the builder.
CHANGELOG.md Records the new functionality.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +372 to +374
os.replace(
get_page_path_fs(self._tome_root, subtype, page), destination
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and fixed in 0b6a037. _publish now writes the destination's manifest as partial, with no pages, before it moves any page file. Only then does it move the pages, write the header copy, and write the complete manifest. An interrupted publish leaves a partial tome, and the next build rebuilds it by default (behavior_if_partial="overwrite"). It never leaves a complete manifest over half-replaced pages. I didn't swap a sibling directory in atomically. On Windows a directory rename can't replace an existing folder, and the swap would still need a delete of the old tome and a manifest rewrite. Invalidating first closes the gap without deleting anything. test_an_interrupted_publish_leaves_the_tome_partial rebuilds a complete multi-page tome with overwrite and stops it after one page file has moved. The tome then reads partial with no pages, and the next default build equals the original. The test fails on the previous commit.

Comment on lines +224 to +225
def _build_tome(self, name) -> bool:
warn_if_invalid_tome_name(name)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and fixed in 0b6a037. tome_names now raises ValueError when a channel's tome name would equal the header tome's name. When tome_name doesn't use {dates}, the check runs in the constructor, before anything is read. Otherwise it runs once the header gives the dates. test_a_channel_tome_may_not_be_the_header covers both cases, and it fails on the previous commit.

billfreeman44 and others added 2 commits September 28, 2026 18:58
… the header's name

- Publishing a staged tome over an existing one first writes the
  destination's manifest as partial with no pages, then moves the page
  files. An interrupted publish now leaves a partial tome, which the next
  build builds again, never a complete manifest over half-replaced pages.
- A tome_name that would give a channel's tome the header tome's name
  raises ValueError: before reading when the name doesn't use {dates},
  otherwise once the header gives the dates.

From the Copilot review.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Conflicts resolved keeping both sides:
- curator.py: build_basic_tomes, then #39's get_dataframe overloads with
  columns= and library=, and TomeCuratorFs.scan().
- CHANGELOG.md: #39's lines stay under 3.3.0; 3.3.0 is tagged, so this
  branch's lines move to a new Unreleased section.
- docs/tome-data-model.md merged without conflict.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@billfreeman44

Copy link
Copy Markdown
Member Author

Merged master into this branch (d9c54a4), which brings in 3.3.0 and #39. I merged rather than rebased, so the PR history stays.

The builder's behaviour is unchanged. Its tests compare frames after reset_index(drop=True), so the loader's new single RangeIndex needed no test changes.

The full suite passes locally in three environments, and pylint and black are clean:

CI is green on every job, test-without-polars included.

@billfreeman44
billfreeman44 merged commit dcfe41c into master Sep 29, 2026
9 checks passed
@billfreeman44
billfreeman44 deleted the worktree-dsdk-review-builder branch September 29, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants