Add AGENTS.md: how to work on rootfilespec by sathabbott · Pull Request #134 · nsmith-/rootfilespec

sathabbott · 2026-09-30T21:28:17Z

🤖 AI generated content

Adds AGENTS.md, the working agreements for coding agents (and people) on this repo. CLAUDE.md imports it, because Claude Code reads CLAUDE.md, not AGENTS.md. It writes down what the recent PRs have followed, so the next contributor or agent doesn't have to rediscover it:

What the package is, and where things live.
Design rules, summarized one line each, with the full text moved into docs/design.md so the two can't drift: objects describe where data is and callers fetch; a value is a Python builtin (strings and key names are bytes) only inside a context that records its ROOT type (a member's annotation, a container's element type, a TKey, or a pointee's Ref), and its decoding must be injective; keep what is on disk; don't guess at what isn't understood.
docs/design.md: "Annotated builtin types vs. objects" now states that context rule (An object slot's class record and the object's own frame are conflated (pointers to non-TObject classes desync) #105 for Ref, the Survey: where each way of writing an object records its ROOT type (wrappers for builtin values) #139 survey), that the choice of builtin is ergonomic (TDatime and TObject written as records of their own (no byte count) cannot be read #123), and the injectivity requirement (Readers that merge distinct on-disk values: kBool, negative char* lengths, long-form short TStrings #140). It replaces the stale pointer note that pointed at FileContext.type_by_name can return an annotated builtin, not only a ROOTSerializable class #135. New sections: keeping what is on disk, content the parser does not understand (Class missing from StreamerInfo: return Uninterpreted (skip by byte count) instead of raising #74), and generated classes (Deprecate Python dataclass annotations as the serialization definition for streamer-generated dataclasses #67, Append version to all class names #22).
Format references: root-io-spec is the reference, including the RNTuple ERRATA.md / NOTES.md; cite it; don't infer the format from this parser.
Setup: the submodule is never updated recursively.
Checks: CI's Format job runs pre-commit run --all-files, at pre-commit's pinned versions, and mypy's pre-commit environment has only pytest, numpy and tomli. Both have tripped PRs.
Tests: a failing-on-main test per change, real fixtures first, case.toml offsets, narrow module names, and the EXPECTED_FAILURES rule.
Issues, commits and PRs: one issue per problem, one PR per issue, Depends on #N for stacked PRs, and a description with Fixes #N, Tests and Breaking changes.
Attribution: the > 🤖 AI generated content header and the Assisted-by: trailer.

It's a proposal: edit anything that doesn't match how you want the repo run. Prettier (--prose-wrap=always) and codespell pass on it.

Assisted-by: claude-code:claude-opus-5-5

codecov-commenter · 2026-09-30T21:30:30Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.78%. Comparing base (66b9290) to head (66fbf03).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #134   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files          37       37           
  Lines        3022     3022           
=======================================
  Hits         2804     2804           
  Misses        218      218

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sathabbott · 2026-09-30T21:43:01Z

@nsmith- what do you think of this? anything you want to add / update from your claude?

nsmith-

I'll paste any updates from local memory in a comment

nsmith- · 2026-09-30T23:31:56Z

🤖 AI generated content

Pushed 3ceaa9c. Here is why each change was made:

Strings as bytes vs. keeping what is on disk (the line-32 comment, from the #127 discussion). The two rules conflict when a string's class is known only from a class tag: for example a TString or std::string read through a pointer, where #127 returns plain bytes. A struct member keeps its encoding in the annotation, and a keyed object keeps it in TKey.fClassName. The pointer case keeps it nowhere, so the value can't be written back as it was read. Writing is a stated goal, so the rules now say:

the ROOT type must stay recoverable, from the annotation or from the enclosing record;
where neither holds it, keep something that does, or document that the value can't be round-tripped;
"keep what is on disk" means everything a writer would need.

The #127 decision itself should go into docs/design.md as a design assumption.

Setup.

pre-commit is not in the dev group, so pre-commit install failed right after the documented install. It is now installed with uv tool install.
CI installs with uv sync from uv.lock, so uv is now the main route and pip is the fallback.

Smaller fixes.

test_spec_cases.py also skips without the submodule.
nox runs both checks.
The test-naming rule now covers only new modules, since test_read.py and test_rntuple_hardcoded.py already break it.
Removed a sentence that repeated the strings-are-bytes rule.

Conventions from the root-io-spec review that were only written down in issues.

New issues are native sub-issues of Tracking: review of bootstrap classes against the root-io-spec reference #66 or Checklist for remaining types in scikit-hep/testdata #9.
Deprecate Python dataclass annotations as the serialization definition for streamer-generated dataclasses #67's direction for dynamic.py: agents shouldn't build more on annotations and inheritance.
Classes missing from the StreamerInfo read as Uninterpreted (Class missing from StreamerInfo: return Uninterpreted (skip by byte count) instead of raising #74).
Generated model names carry the checksum.

RooFit is left out on purpose: the updated root-io-spec now specifies it, so it is no longer a documented skip (#71 needs updating).

Attribution. Claude Code's default adds a Co-Authored-By trailer and a "Generated with Claude Code" footer. A contributor's private settings can override that, but the repo's rules didn't. AGENTS.md now says Assisted-by: replaces both.

Assisted-by: claude-code:claude-opus-5-5

The design note claimed a streamed object's class tag keeps a string's type, like an annotation or a key's class name does. It doesn't: read_streamed_item reads the tag to look the type up and then returns bare bytes. Members, container elements and keyed records keep the type; a string read through a pointer can't be written back as read. Tracked in nsmith-#135, as Nick noted on nsmith-#127 and in nsmith-#134's AGENTS.md. Assisted-by: claude-code:claude-opus-5-5

sathabbott · 2026-10-01T01:04:48Z

🤖 AI generated content

Thanks for 3ceaa9c. I checked AGENTS.md against the repository and pushed e931d05 with the corrections below, so an agent reading it doesn't assume something the code doesn't do yet:

Setup: there's no uv.lock; it's in .gitignore (line 166). CI installs with uv sync --group test (ci.yml), so the comment now says dev is CI's test group plus mypy.
Class missing from StreamerInfo: return Uninterpreted (skip by byte count) instead of raising #74 and Append version to all class names #22 are still open, so those two rules now read as planned. Today a class missing from the StreamerInfo raises Unknown type ... (several test_read.py xfails carry that error), and a generated class has the bare class name (class Hit(StreamedObject)); only the generated module's name is hashed.
Tests and the submodule: besides test_spec_fixtures.py and test_spec_cases.py, test_compressed_size.py (from Fix TKey's compression test (#77), refuse RBlob keys, and the container items of #87 #130) also uses root-io-spec fixtures, and it fails without the submodule rather than skipping. I checked by running the suite from a copy with no submodule: those 4 cases are the only failures. The text now says to check the submodule out first, and that a test that doesn't skip is a bug. The skip is Skip test_compressed_size.py without the root-io-spec submodule #136.
Smaller: case.toml files are under gen/written/ as well as gen/cases/. "No I/O" now says "parsing code", since reader.py does read files. And new issues become sub-issues "where a tracker fits": the RNTuple issues ([rntuple] Page locations give no global cluster ID across cluster groups #116–[rntuple] Support non-standard locator types 0x02 (Object64) and 0x7e #122) fit neither Tracking: review of bootstrap classes against the root-io-spec reference #66 nor Checklist for remaining types in scikit-hep/testdata #9.

I also added Depends on #127 to the description, since the string rule describes ROOTString.

…#127) * Read RNTuple strings as plain bytes docs/design.md promises that where a builtin Python type captures a ROOT type, the deserialized member is that builtin: bytes for strings. RNTuple strings were RString wrappers instead. Add CountedString, a MemberSerDe for a string stored as its length (in a given struct format) followed by that many bytes, read as plain bytes. An RNTuple string is Annotated[bytes, CountedString("<I")], exported from rntuple.schema as RNTupleString, and the eight RNTuple string members (the header's name, description and library; a field's name, type name, type alias and description; extra type information's type name) use it. RString is removed. This is the RNTuple row of #68. The TString, std::string and char* rows stay open there; char* (#20) can use CountedString(">i"). Part of #68 Assisted-by: claude-code:claude-opus-5-5 * Rename tests/test_rntuple_strings.py to tests/test_counted_string.py Name the module after what it tests, so it does not read as the home of a broad set of tests (review on #125). Assisted-by: claude-code:claude-opus-5-5 * Compare header names with the key names as bytes, after #130 Assisted-by: claude-code:claude-opus-5-5 * Read every ROOT string type as plain bytes with ROOTString (rest of #68, #20) Replace CountedString with ROOTString(encoding), one Literal value per on-disk form (root-io-spec Conventions §5), as suggested in review: - "RNTuple": u32 little-endian length, then the bytes - "TString": the counted string, one length byte or 255 then a u32 - "std::string": a std::string data member, the counted string inside a byte count and version word (§5.3); the byte count is checked - "charstar": a char* member, i32 length then the bytes, no 255 escape; a length <= 0 reads as b"" (§5.4, #20) TString is now the alias Annotated[bytes, ROOTString("TString")], so every TString member is bytes and .fString is gone. STLString is removed. read_string(buffer, encoding) reads one string by hand (TKey, TList options). Generated classes write the annotation out: TStreamerString emits ROOTString('TString'), TStreamerSTLstring ROOTString('std::string'), a kCharStar basic type ROOTString('charstar'), and cpptype maps string, std::string and TString elements of containers and pointees to ROOTString('TString'). A string stored as an object of its own (a key, or through a pointer) is looked up by class name, so TString, TStringLong and string stay resolvable by name, and serializable.read_value reads either a class or an annotated builtin. TStringLong is the "charstar" encoding (§5.1.1). As a result, root-io-spec's serialization/stringlong.root now reads, and serialization/unframed-records.root reads its TDatime, TString and TStringLong records and stops at the TObject record (#123). Assisted-by: claude-code:claude-opus-5-5 * Make a std::string member's frame a ROOTString option, not an encoding A std::string data member is the TString encoding inside the usual byte count and version word (root-io-spec Conventions §5.3). The frame is not a fourth string encoding, so: - StringEncoding is back to the three length formats Nick listed on #127: "RNTuple", "TString", "charstar". - ROOTString(encoding, framed=False). With framed=True it reads the frame with StreamHeader, as StdVector and the other framed members do, and checks the byte count, instead of parsing the frame itself with a second copy of kByteCountMask. - read_string() is gone. ROOTString.read(buffer) is the one string reader: the member reader calls it, and TKey and TList call it for their hand-read strings. - read_value() takes type[MemberType] | type[ROOTSerializable], as Nick suggested, and goes through _build_read for both kinds of type. - docs/design.md states the design assumption Nick raised: the bytes don't record their encoding; the annotation, key class name or class tag that holds them does, and writing a string back needs it. - test_tdatime_record is dropped: the TDatime record reads as a side effect of read_value, but it belongs to #123, not #68. Assisted-by: claude-code:claude-opus-5-5 * Read a TypedTKey's object with read_value, like an untyped key's TypedTKey.read stores the looked-up type as objtype, and after #68 a lookup of "TString", "TStringLong" or "string" returns an annotated alias, not a class. read_object called objtype.read() on it and raised AttributeError, so a TypedTKey of a string record could not be read, although fetching the same key untyped returned its bytes. Both branches now read through read_value. Test: the "tstring" record of root-io-spec serialization/unframed-records through TypedTKey reads b"hello". Assisted-by: claude-code:claude-opus-5-5 * Fix the update_members typo in docs/design.md "read_mupdate_membersembers" was a garbled "update_members", as Nick noted on #127. Assisted-by: claude-code:claude-opus-5-5 * Say that a string read through a pointer loses its ROOT type The design note claimed a streamed object's class tag keeps a string's type, like an annotation or a key's class name does. It doesn't: read_streamed_item reads the tag to look the type up and then returns bare bytes. Members, container elements and keyed records keep the type; a string read through a pointer can't be written back as read. Tracked in #135, as Nick noted on #127 and in #134's AGENTS.md. Assisted-by: claude-code:claude-opus-5-5 * Compare key class names as bytes in the page checksum tests, after #128 #128's tests index key.fClassName.fString; after #68 the class name is plain bytes. Assisted-by: claude-code:claude-opus-5-5

What the package is and its layout, the design rules from docs/design.md and recent reviews (locators, strings as bytes, keep what is on disk, don't guess), root-io-spec as the format reference, setup with the non-recursive submodule, the checks CI runs (pre-commit's pinned tools, and mypy's minimal environment), how tests are written, the shape of issues, commits and PRs, and the AI-attribution convention. CLAUDE.md imports it, since Claude Code reads CLAUDE.md. Assisted-by: claude-code:claude-opus-5-5

…ventions The strings-as-bytes rule and "keep what is on disk" conflict when a string's class (TString vs std::string) is known only from a class tag that plain bytes drop, e.g. a string read through a pointer (nsmith-#127): the value can then not be written back as read. The rules now say the ROOT type must stay recoverable from the annotation or the enclosing record, or the loss must be documented. Setup: CI installs with `uv sync` from uv.lock, and pre-commit is not in the dev group, so `pre-commit install` failed after the documented install. The pip route stays as the fallback. Also: test_spec_cases.py skips without the submodule too; nox runs both checks; the naming rule is for new test modules; new issues are sub-issues of nsmith-#66 or nsmith-#9; nsmith-#67's direction for dynamic.py; the Uninterpreted default for missing classes (nsmith-#74) and checksummed model names; and Assisted-by replaces tools' default attribution trailers. Assisted-by: claude-code:claude-opus-5-5

- Setup: there is no uv.lock (it is gitignored), and CI installs the test group (`uv sync --group test`); dev is that group plus mypy. - The Uninterpreted rule (nsmith-#74) and checksum-named models (nsmith-#22, under nsmith-#67) are open issues: today a missing class raises "Unknown type ...", and a generated class has the bare class name. Say they are planned. - Tests: test_compressed_size.py (from nsmith-#130) also uses root-io-spec fixtures and fails without the submodule instead of skipping; say to check it out first, and that a test that doesn't skip is a bug. - case.toml files are under gen/cases/ and gen/written/. - "No I/O" is about the parsing code; reader.py does read files. - Sub-issues: only where a tracker fits (the RNTuple issues fit neither nsmith-#66 nor nsmith-#9). Assisted-by: claude-code:claude-opus-5-5

…ve decoding Rewrite the builtin-types rule after nsmith-#127's review: a value may be an annotated builtin only inside a context that records its ROOT type and encoding (a member's annotation, a container's element type, a TKey's fClassName, or a pointee's Ref, which is to keep the stream header per nsmith-#105). Decoding must be injective on every accepted input, so inputs that would decode to the same value are rejected or kept, not merged. Assisted-by: claude-code:claude-opus-5-5

AGENTS.md and docs/design.md described the same rules and had already drifted: design.md sent the pointer case to nsmith-#135, AGENTS.md to nsmith-#105. docs/design.md now holds each rule in full, with the reasons and the issues still open against it: - "Annotated builtin types vs. objects" states the context rule (member annotation, container element type, TKey, pointee Ref per nsmith-#105, survey in nsmith-#139), that the choice of builtin is ergonomic (TDatime, nsmith-#123), and the injectivity requirement (kBool 0x99, root-io-spec ElementTypes §2.5; nsmith-#140). - New sections for keeping what is on disk, content the parser does not understand (nsmith-#74), and generated classes (nsmith-#67, nsmith-#22). AGENTS.md keeps one line per rule, linking its design.md section, so an agent still has the rules in context. Assisted-by: claude-code:claude-opus-5-5

nsmith-

I am happy with this now. @sathabbott if you are as well let's merge

nsmith- reviewed Sep 30, 2026

View reviewed changes

Comment thread AGENTS.md Outdated

nsmith- mentioned this pull request Sep 30, 2026

Read every ROOT string type as plain bytes with ROOTString (#68, #20) #127

Merged

sathabbott force-pushed the docs/agents-md branch from 07df49a to e931d05 Compare October 1, 2026 01:05

sathabbott mentioned this pull request Oct 1, 2026

Skip test_compressed_size.py without the root-io-spec submodule #136

Merged

sathabbott and others added 4 commits October 1, 2026 14:32

nsmith- force-pushed the docs/agents-md branch from e931d05 to 131c91a Compare October 1, 2026 19:42

nsmith- approved these changes Oct 1, 2026

View reviewed changes

sathabbott mentioned this pull request Oct 1, 2026

An object slot's class record and the object's own frame are conflated (pointers to non-TObject classes desync) #105

Open

nsmith- merged commit 9b6216d into nsmith-:main Oct 1, 2026
9 checks passed

sathabbott deleted the docs/agents-md branch October 1, 2026 21:05

Navigation Menu

Repository navigation

Add AGENTS.md: how to work on rootfilespec - #134

Add AGENTS.md: how to work on rootfilespec#134
nsmith- merged 5 commits into
nsmith-:mainfrom
sathabbott:docs/agents-md

sathabbott commented Sep 30, 2026 •

edited by nsmith-

Loading

Uh oh!

codecov-commenter commented Sep 30, 2026 •

edited

Loading

Uh oh!

sathabbott commented Sep 30, 2026

Uh oh!

nsmith- left a comment

Uh oh!

Uh oh!

nsmith- commented Sep 30, 2026

Uh oh!

sathabbott commented Oct 1, 2026 •

edited

Loading

Uh oh!

nsmith- left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sathabbott commented Sep 30, 2026 • edited by nsmith- Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sathabbott commented Sep 30, 2026

Uh oh!

nsmith- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nsmith- commented Sep 30, 2026

Uh oh!

sathabbott commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nsmith- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sathabbott commented Sep 30, 2026 •

edited by nsmith-

Loading

codecov-commenter commented Sep 30, 2026 •

edited

Loading

sathabbott commented Oct 1, 2026 •

edited

Loading