Boot config at .config/<repo>/config.yaml, deployment docs, natural-alignment pin - #908
Conversation
…cs + natural-alignment pin Three deliverables, one arc: 1. `soa_config` module — a deployment reads ONE YAML object from its own bucket at `.config/<repo-name>/config.yaml` declaring which bakes exist and which hydrate to local disk. Same binary, same behaviour, dev container and Railway alike, because both read the same object. Boot config read once at startup — NOT a hot-path serialization surface, and documented as such so nobody "fixes" it. The overwrite doctrine is config-as-pointer: an existing table is never silently overwritten (OnExisting::Refuse default); a refresh writes a NEW timestamped table and flips one YAML line, because S3 has no atomic rename — renaming an N-object Lance dataset is N copies + N deletes, non-atomic, and a crash mid-rename leaves the dataset split with no valid pointer. 16 unit tests, each validation rule with a genuine can-fire case, plus a test that the SHIPPED example parses through the REAL parser (include_str!), so example and schema cannot drift apart silently. 2. Deployment docs — docs/SOA_BAKE_DEPLOYMENT.md (bake -> soa_to_lance -> table header contract -> the stride/full-zip verbatim mechanism with the corrected causality -> the two serving patterns with measured, scope-limited numbers -> what is still NOT measured) and docs/S3_LAYOUT.md (the bucket map: .config/<repo>/, ledger prefixes, _tests/ scratch, the AWS_ENDPOINT vs AWS_ENDPOINT_URL trap, the refresh/purge lifecycle). Example config uses REAL minted geo classids (0x0F01..03) — an earlier draft invented "0x0D01 ontology", which is actually hr_employee in the HR domain; the near-miss is recorded in the example itself as the reason for the never-invent-a-classid rule. 3. soa_verbatim alignment tightened 64 -> NATURAL (off % 512 == 0). The 64 came from align_of::<NodeRow>(); natural alignment is the stronger claim the measurement (offset 0 on the Iceland bake) already supports, and it is the one that matters operationally: a 512-aligned 512-byte row arithmetically cannot straddle a 128-bit SIMD lane, a 128-byte cache line, or a 4 KiB page/NVMe sector — 8 whole rows per page, zero padding, no half-lanes ever. Alignment constrains the run's START only; each row stays one contiguous unit (that is assertion 1). Gate: soa_config 16/16, soa_verbatim 6/6 (both S3 arms live against the real endpoint), clippy clean, fmt clean, example YAML validated through both the Rust parser and an independent YAML load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b9ee2b47-7d2e-4c72-bb62-04b8802e8f88) |
…evel The same three questions recurred across this arc (64 bit or 64 byte? does align(64) fragment the row? do half-lanes exist?), and three separate AI research dumps each answered them partially wrong in a DIFFERENT way. This primer settles the questions once, names the specific errors so the next person consulting the same tools can grade them, and labels every claim [MEASURED] / [SOURCE file:line] / [ARITHMETIC]. Core content: the three-numbers table (64-bit metadata word vs 64-byte cache line/type alignment vs 512-byte natural row alignment); alignment constrains the START address, never granularity; the straddle arithmetic (a 512-aligned 512-byte row cannot cross a 16 B lane, 64/128 B line, or 4 KiB sector — and alignment of row 0 propagates through the stride to every row forever); why Lance versioning cannot shift data bytes (immutable data files, separate manifest objects — both measured); the per-file-not-per-row cost bound; and the table mapping each property to the red-turning assertion in tests/soa_verbatim.rs that keeps it true. Deliberately shipped THROUGH the PR review loop: if any claim is wrong, CodeRabbit/Codex attack it here — the same loop that produced eight real findings on #907. A claim in a reviewed doc beats a claim in a chat. Cross-linked from docs/SOA_BAKE_DEPLOYMENT.md header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
![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: 53e3aad363
ℹ️ 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".
| # two entries pointing at one table means one | ||
| # of them is either reading garbage or racing | ||
| # the other's writes. | ||
| classid: "0x0F01" # Hex classid (0x-prefixed) identifying this |
There was a problem hiding this comment.
When an operator copies this example, 0x0F01 is interpreted as the complete classid even though the active contract stores the canonical 0xDDCC half in the high 16 bits (compose_classid(0x0F01, 0) is 0x0F01_0000; see crates/lance-graph-contract/src/ogar_codebook.rs). With the value shown here, classid_canon(0x0000_0F01) returns zero, so a reader routes the Geo bake as Reserved/default and may select the wrong ClassView—the exact failure the adjacent comment warns about. Supply the composed eight-digit classids, or explicitly model and name this field as a u16 concept ID rather than a classid.
Useful? React with 👍 / 👎.
| #[derive(Debug, Clone, PartialEq, serde::Deserialize, serde::Serialize)] | ||
| pub struct BakeEntry { |
There was a problem hiding this comment.
When a deployment config contains a typo such as slab_digset or hydarte, Serde silently ignores it because BakeEntry does not use deny_unknown_fields; the corresponding default then turns the digest into None or hydration into false. That makes a supposedly validated boot config silently drop its slab/sidecar integrity pin or change serving behavior instead of failing loudly, so unknown entry fields should be rejected during parsing.
Useful? React with 👍 / 👎.
|
@coderabbitai review (Head moved mid-review — Generated by Claude Code |
|
|
AdaWorldAPI
changed the title
…2 on #908) Two P1s from Codex, both real, both now enforced invariants rather than comments — this field had ALREADY been corrected once in #908 (for naming the wrong domain) and still shipped wrong in WIDTH, so a doc-comment warning has demonstrably not been sufficient. P1-1 — concept id written where a u32 classid belongs. CLASSID_ORDER is CanonHigh, so a classid is compose_classid(canon, custom): canon (the minted concept 0xDDCC — domain in the MOST-SIGNIFICANT byte, which is what makes classids sort/prefix-search hierarchically) in the HIGH half, the app render prefix in the LOW half. The example config carried the bare concept "0x0F01", which lands it in the LOW half — classid_canon() then returns 0 and every reader routes the bake as Reserved/default, the exact "total class collapse" that function's own doc warns about. Verified from source, not taken on the reviewer's word. Fix: parse() now parses the classid as a real u32 and rejects a zero canon half with ConceptIdAsClassid { found, suggestion }, where the suggestion is the composed form (0x0F01 -> "0x0F010000") so the operator is told what to write, not merely that they were wrong. New BakeEntry::classid_u32() so callers stop re-parsing the string and re-deciding what a malformed one means. Example config and every test fixture moved to composed 8-hex classids. P1-2 — unknown keys silently ignored. A typo'd `slab_digset` or `hydarte` was dropped by serde and the field defaulted: digest pin gone, hydration silently off, in a config whose entire purpose is failing loudly at boot. Fix: #[serde(deny_unknown_fields)] on BakeEntry and SoaConfig. Both falsifiers verified by the disable-the-fix run: with the canon check stubbed to `if false` and both deny_unknown_fields removed, exactly the two new tests go red (16 passed / 2 failed) and no others — so neither is passing for an unrelated reason. Each is two-sided: the bad shape is rejected AND the good shape is accepted with its value intact, so the rules cannot pass by rejecting everything. Note the cross-check earned its keep: the_shipped_example_config_parses_ through_this_parser stayed green only because the example was corrected alongside the parser — fixing one without the other would have gone red. 18/18 soa_config, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
2 participants
Follow-up to #907, completing the deployment story around the SoA→Lance path.
1.
soa_config— the config IS the pointerA deployment reads one YAML object from its own bucket at
.config/<repo-name>/config.yamldeclaring which bakes exist and which hydrate to local disk. Same binary behaves identically in a dev container and on Railway because both read the same object. Read once at boot — explicitly documented as not a hot-path serialization surface (same category as the contract crate'sbuild.rsmanifest parse), so a future session doesn't "fix" it.Overwrite doctrine:
OnExisting::Refuse(default) — an existing table is never silently overwritten.NewVersion— a refresh writes a new timestamped table and flips one YAML line. Why not rename-to-OLD: S3 has no atomic rename; renaming an N-object Lance dataset is N copies + N deletes, non-atomic, and a crash mid-rename leaves the dataset split across two prefixes with no valid pointer. Flipping the config line is O(1) and reader-atomic.16 unit tests — every validation rule has a genuine can-fire case — plus
the_shipped_example_config_parses_through_this_parser(include_str!), so the example and the schema cannot drift apart silently.2. Deployment docs
docs/SOA_BAKE_DEPLOYMENT.md— bake →soa_to_lance→ header contract → the corrected verbatim causality (stride > 256-byte mini-block cutoff ⇒ full-zip; the compression metadata is a backstop, not the cause) → the two serving patterns with measured, scope-limited numbers → an honest "what is NOT measured" section (request count; P-CACHE-1).docs/S3_LAYOUT.md— the bucket map:.config/<repo>/, per-repo ledger prefixes,_tests/scratch, theAWS_ENDPOINTvsAWS_ENDPOINT_URLtrap, refresh/purge lifecycle (purge is the only destructive op and never automatic).examples/soa-config.example.yaml— heavily commented; uses real minted geo classids (0x0F01..03). An earlier draft invented "0x0D01ontology" — which is actuallyhr_employeein the HR domain. The near-miss is recorded in the example itself as the justification for the never-invent-a-classid rule.3. Alignment pinned at NATURAL (512), up from 64
The 64 came from
align_of::<NodeRow>()— sufficient for the cast, silent about straddling. Natural alignment is the stronger claim the measurement (offset 0 on the Iceland bake) already supports, and the one that matters operationally: a 512-aligned 512-byte row arithmetically cannot straddle a 128-bit SIMD lane, a 128-byte cache line (Apple M-series), or a 4 KiB page/NVMe sector — 8 whole rows per page, zero padding, no half-lanes ever. Alignment constrains the run's start only; each row stays one contiguous unit.Gate
soa_config16/16 ·soa_verbatim6/6 (both S3 arms live against the real endpoint) · clippy clean · fmt clean · example YAML validated through both the Rust parser and an independent YAML load. Key names only in every committed file; values come from the deployment environment.🤖 Generated with Claude Code
https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
Generated by Claude Code