feat(store): derive per-record revision counts from the operation log - #164
Conversation
The append-only log already names every change to every record key, and a key appears at most once per operation, so how often a stored record was rewritten is already recorded and was simply never read back. Add a read-only derivation and nothing else: - `reduceOhRecordRevisionsV1` reduces one key's bounded, unordered log changes into counts: puts, tombstones, puts after the first, distinct put digests, the latest change, and the head sequence read through. It parses each change from `unknown`, sorts by sequence, and rejects two changes to one key in one operation, a change ahead of its through sequence, and more than 65,536 changes. - `ohRecordRevisionChangesFromOperationsV1` collects those changes from operations a reader already holds, bounded at 65,536 operations, so a change-feed consumer derives identical counts without local SQL. - `OhSqliteStore.recordRevisions` reads them from the local log in one read transaction, newest first, under an explicit limit. The existing `UNIQUE(operation_sha256, record_key)` index already serves the query, so no table, index, or migration is added. The kernel reports counts and attaches no meaning to them. No threshold, ranking, or trust judgement is encoded here; that is an application's work. Comparing the put count with the distinct-digest count separates a rewrite from a content change, and truncation drops the oldest changes so the latest change and through sequence stay exact while the counts become lower bounds. Operation bytes, digest preimages, record kinds, identifier grammars, applied migrations, and the `OhStoreV1` port are unchanged; both readers are additive and never write. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Independent review found no incorrect derivation but three claims and bounds worth correcting. - `operationsPerRead` drops from 65,536 to 1,000, matching the operation page and import bounds. Every element is re-verified through `parseOhOperationV1`, which recomputes canonical JSON and a digest over an operation bounded at 64 MiB, so the old bound admitted seconds of synchronous work that no store in this repository can even produce a page for. - `ohRecordRevisionChangesFromOperationsV1` refuses a feed that spans two spaces. A sequence numbers an operation within one space, so two spaces sharing a record key previously reached the reducer as an apparent duplicate sequence and failed with a message that diagnosed the wrong thing. That message now names what it found: two changes at one sequence. - `spec/v1/store.md` no longer implies that `limit` bounds the search. It bounds the result; the read costs one index probe per operation in the space and stops early once the limit is filled, so a rarely written or absent key is proportional to the log rather than to `limit`. Measured numbers are stated. An index on the change table's record key still does not help — with or without one, SQLite drives the operation index for the space filter and the sequence ordering, and an absent key over 20,000 operations costs 16 ms either way — so no migration is added. Also: the put-count against distinct-digest-count comparison separates a rewrite from a content change only while no tombstone touched the key, since removing and restoring identical bytes changes the record twice while storing one digest. Both the type doc and the specification now say so. Tests: a feed spanning two spaces, the operation bound, a truncated read whose newest change is a tombstone, two spaces sharing one database file (the join to `oh_operations` is the only thing scoping this read), a history that arrived by import rather than local commit, and a property test for the change parser and the ordering law. The wall-clock assertion is gone; it could not support the claim its test name made, and CONTRIBUTING.md discourages depending on timing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the feed read Re-review caught that the cost paragraph shipped in 2c57242 was false as a general claim, and I reproduced that myself. My earlier measurement compared an index on the change table's record key against no index on a database with no table statistics, then generalized from it. Adding `ANALYZE` reverses the outcome: the plan flips to `SEARCH oh_operation_records USING INDEX (record_key)` with a temporary B-tree for the ordering, and the absent-key read goes from 16 ms to 0.036 ms on a 20,000-operation space. `ANALYZE` alone, with no schema change, instead makes the frequently written key at `limit` 1 slower by a factor of about forty. Nothing in this repository runs `ANALYZE`, but a host, an operator, or `PRAGMA optimize` can, so the read's speed was resting on the absence of statistics while the specification asserted it as a property. A versioned contract should not assert a planner outcome that one `ANALYZE` reverses, and it should not carry millisecond figures from one machine, one record size, and one key density with no test gating them. The paragraph now states only what V1 promises: `limit` bounds the result, not the search; the cost grows with the length of the log rather than with `limit`; a rarely written or absent key is the expensive case; and the plan depends on the indexes and statistics the database carries, so latency is something to measure rather than something the contract grants. The figures stay in commit messages, where they belong. `ohRecordRevisionChangesFromOperationsV1` now takes the space explicitly and requires one contiguous run of operations in sequence order. The previous guard proved a feed was internally consistent, not that it was the right space: an entirely wrong space passed every check and returned plausible wrong counts, which inferring the space from the first operation cannot detect in principle. A feed missing a page was worse — it lowered every count and still reported `truncated: false`. Both are now rejected by name. Contiguity between separate calls stays the caller's to maintain, and the doc comment says so. The API is unreleased, so the required argument costs nothing now and a version later. The property test now replays the changes as a state machine rather than recounting them with the implementation's own expressions, and checks that the replayed end state agrees with the reported latest change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g rewrites Independent review found the feed reader reporting lowered counts as exact. Three fixes, each with the test that reproduces the defect. A missing leading page was undetectable. The contiguity guard checked adjacency within the array it was handed, so a run that was internally contiguous but did not begin at the log's first operation was accepted, and its lowered counts were labelled `truncated: false`. A consumer following a live feed from its cursor is always in that position. Worse, the output was indistinguishable from a correct one: a partial window over a key written at 1..6 returned the same shape as an exact read of a key genuinely first written at 4. The reader now returns the first sequence it observed, `reduceOhRecordRevisionsV1` requires it, and a run that did not start at sequence 1 reports `truncated: true`. The two cases are distinguishable again. Comparing `puts` to `distinctPutDigests` did not mean what the specification said. A key put as A, B, A holds three puts and two digests with no tombstone while every put changed the record, and is indistinguishable by those numbers from A, A, B, which contains one genuine rewrite. `idempotentPuts` now counts puts whose digest equals the immediately preceding put's, which is the quantity that carries the claim; a put restoring bytes after a tombstone changes the record and does not count. The specification no longer assigns that meaning to the digest count. `truncated` was the one input silently defaulted, so a misspelling returned an exact-looking answer. The reducer now requires exact keys, matching `parseOhRecordRevisionChangeV1` beside it. A dead per-iteration change bound in the feed reader is removed: one change per key per operation and a 1,000 operation cap make 65,536 unreachable. Left alone, and reported rather than fixed: a receipted whole-space purge surfaces from this read as `OhIntegrityError` via `head()`, as it does from `snapshot()` and `verify()`. It is a pre-existing pattern, not introduced here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed independently of the implementation, and the gates were re-run rather than taken from a report. The independent review earned its place. It found four defects, the serious one being that a change-feed reader missing its leading page reported lowered counts as That is worth naming precisely, because it is the same failure the measurements behind this work keep finding. A signal that is wrong is survivable. A signal that is wrong and indistinguishable from being right is the one that does damage, because no downstream reader can discount it. Same reason the lexical support check was worse than useless at 0.54x rather than merely uninformative. A claim in this description was wrong and is corrected in place. It asserted that an index on the change table's record key does not change the plan or the cost. That was measured on a dense key where every operation touches it, so the scan is already optimal. On a sparse key — 20,000 operations over 1,000 keys, the realistic shape — the index is worth two to three orders of magnitude (21.0 ms to 0.029 ms on an absent-key read). The conclusion is reframed rather than deleted: no migration ships because the index is the host's decision, not because it would not help. The reviewer and I also disagree on the direction for Verified here, not reported: Merging with one limitation on the record. The counts follow a single record key. Where an application models a correction as a new record superseding an older one, this signal under-reports — and that is how oh's own observation profile represents corrections, so it is the likely shape in practice, not an edge case. This lands the derivation that is exact and cheap; the supersession-chain read is separate work and is not pretended to be done here. |
This branch was successfully deployed
1 participant
The append-only log already names every change to every record key, and
canonicalKnowledgeGraphChangesV1makes a key appear at most once peroperation. How often a stored record was rewritten was therefore already
recorded and simply never read back —
revisionCount|timesRevised|updateCountmatched nothing in
src/.The motivation is a measurement, not a hunch. Across two external curation
corpora, a model's own coarse self-reported confidence did not predict its own
errors (1.21x and 1.03x lift, both intervals spanning 1.0), and a third party's
published
highconfidence label failed to predict even its ownrelease-to-release revision (8.8% [6.7–11.4] vs 9.7% [8.5–11.0], n=14,560).
What did survive needed no external answer key: scoring a system against its
own prior output. This store already holds that prior output.
What this establishes
puts and tombstones per key, in sequence order, with digests.
Measurements, and a corrected claim
An earlier revision of this description asserted that adding an index on the
change table's record key "leaves the plan byte-identical and does not improve
the time." That was wrong, and it is worth stating plainly because it was the
stated justification for shipping no migration.
It was measured on a dense key: 2,000 operations x 50 changes where the key
appears in every operation, so scanning every operation is already optimal. At
that shape the index genuinely changes nothing (11.0 ms vs 10.2 ms, inside noise).
Re-measured on a sparse key — 20,000 operations over 1,000 keys, which is the
realistic shape — the index dominates:
limit: 1An independent review measured the same direction at that scale and also found a
case where
limit: 1regressed with the index; that did not reproduce here,which is itself the point. The cost depends on the indexes and table statistics
the host database happens to carry and can move in both directions, so this
specification states the bound on the result and does not promise a plan. No
migration is added because choosing an index is the host's decision, not because
an index would not help.
What is stable across every run: the
limitbounds the result, not the search.In the worst case the read examines every operation in the space up to its
through sequence, so cost grows with the length of the log, and a key written
rarely or not at all is the expensive case.
Nothing in CI guards any of these numbers. They come from probe scripts, not
committed benchmarks.
What this does not establish
judgement is encoded; whether a revised claim deserves review is an
application's decision, and that split is deliberate.
as a new record superseding an older one — as the observation profile does via
OhObservationValueV1.supersedes— the correction is a separate key with itsown counts. Following that chain is a different, values-level derivation and is
not implemented here. For a consumer whose corrections take that shape, this
signal will under-report, and that is the more likely shape in practice.
truncated: truemeans the counts are lower bounds. Truncation drops theoldest changes, so
latestKind,latestSequenceandthroughstay exact.that any version of it was right.
Surface
src/store.ts:OH_RECORD_REVISIONS_LIMITS_V1,OhRecordRevisionChangeV1,OhRecordRevisionsV1,parseOhRecordRevisionChangeV1,reduceOhRecordRevisionsV1,ohRecordRevisionChangesFromOperationsV1.src/sqlite/store.ts:OhSqliteStore.recordRevisions(key, { limit }), oneread transaction, newest first, bounded.
spec/v1/store.mdplus its byte-identicalsite/public/spec/v1/store.mdmirror;
dist/rebuilt.Additive and read-only. Operation bytes, digest preimages, record kinds,
identifier grammars, applied migration SQL,
OH_SQLITE_SCHEMA_VERSION, and theOhStoreV1port are unchanged. No new table, index, or cost surface. Neitherreader writes. The SQL path and the change-feed adapter feed the same reducer,
and a test asserts the two agree, so they cannot drift.
Verification
bun run typecheck,bun run check:effect,bun run check:cost-surfaces(21 surfaces),
bun run build(byte-reproducible, no untracked files), andfocused tests in
src/store.test.ts,src/sqlite/store.test.ts,src/sqlite/port.test.ts,src/sqlite/migrations.test.ts,src/observe.test.ts— 69 pass, 0 fail. Spec mirror confirmed byte-identical with
diff.One failure in the full local
srcsuite is a 5,000 ms subprocess-spawn timeout(
exit 143) insrc/cli.test.ts:136, a test that spawns ~30 CLI subprocessesand references no symbol this branch changes. CI on this head supplies the
source aggregate, which has not been run locally.
Independent review
Reviewed adversarially against a later commit, with counterexample probes rather
than a read-through. Four defects were found and are fixed in
fb57d1a:contiguity guard checked adjacency within the array it was given, so a run
starting at the consumer's cursor was accepted and labelled
truncated: false,indistinguishable from an exact read of a key first written later. Readers now
return the first sequence observed, and a partial window reports a bound.
putsvsdistinctPutDigestsdid not separate a rewrite from a contentchange. A, B, A and A, A, B produce identical numbers while only the second
contains a rewrite.
idempotentPutsnow carries that claim.truncatedwas silently defaulted, so a misspelled key returned anexact-looking answer. The reducer requires exact keys.
The review also confirmed, by construction and probe:
revisions = puts - 1is avalid lower bound under truncation with no counterexample; no off-by-one in the
limit + 1truncation probe; space isolation is clean across two stores on onefile; the read mutates nothing (verified by dumping every table before and after,
including a throwing call); and no policy or trust judgement is encoded.
One finding was left unfixed and is reported instead: a receipted whole-space
purge surfaces from this read as
OhIntegrityErrorviahead(), exactly as itdoes from
snapshot()andverify(). Pre-existing pattern, not introduced here.Open items
🤖 Generated with Claude Code