Skip to content

feat(spine): recompute the temporal spine from the log, and name the gap (#1283) - #1336

Merged
github-actions[bot] merged 10 commits into
mainfrom
feat/issue-1283-spine-recompute
Aug 5, 2026
Merged

github-actions[bot] merged 10 commits into
mainfrom
feat/issue-1283-spine-recompute

Conversation

@robotrocketscience

@robotrocketscience robotrocketscience commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Refs #1283 — AC2's recompute half, on the operator-funded scope. Does not close the issue: the writer change (item 3 of the corrected ruling) and AC4's non-increasing fan-in assertion are follow-ups.

Stacked on #1331, which lands benchmarks/ingest_log_ulid_clusters.py — referenced by the CHANGELOG entry and by the module docstring as the evidence for the synth rule. No code dependency; merge order doesn't matter for correctness, only for the reference.

This reports a gap, not drift, and the command says so

The ratified contract keys the recompute on (created_at, ingest_log ULID). The shipped writer orders by (created_at, rowid) — and rowid is exactly the problem: it is implicit and VACUUM may renumber it, which is why the contract picked the log's ULID over anything read off the belief table.

The writer has not moved yet, so a zero divergence here would mean the recompute had been fitted to the defect. That's why nothing in the tests asserts zero on a real store, and why aelf spine verify exits 0 regardless and prints the distinction — "93.68%" on its own reads as decay.

shipped TEMPORAL_NEXT : 41,929
recomputed            : 41,382
reproduced            : 39,280 (93.68%)
--- misses, by cause ---
  no-log endpoint     : 2,100 (unreconstructible — the ordering was never durable)
  fan-in > 1          : 546 (writer defect; expected to be non-increasing)
  other               : 3 (the only bucket a key disagreement moves)

Those reproduce the issue's gate measurement and its correction to the digit, from a clean-room implementation — 93.68%, 2,100 / 546 / 3. Three sessions have now landed on the same decomposition by different routes.

Three rules, none of them a heuristic

**Synth rows excluded by source_kind = 'legacy_unknown'.'** A stated durable column, exact in both directions on this store. The #263 synthesis minted 20,852 rows inside a 201 ms window whose order is beliefs.rowid` relabelled; honouring their ULIDs would launder rowid order into the key the contract calls durable.

Two heuristic detectors were measured first and both failed loudly, which is why this one keys on a column:

detector outcome
prefix disagrees with ts by >1 day catches legitimately delayed derivation — 30,738 rows flagged, reproduction collapses to 34.75%
exclude the largest ULID-prefix date cluster drops 51.8% of the log — that cluster is a real bulk backfill carrying 20,095 session-scoped beliefs
source_kind = 'legacy_unknown' exact both ways; no threshold, no drift as the log grows

Earliest qualifying log row wins. Later rows are corroborations; only the first records insertion.

No-log beliefs sort last within their created_at group, then by id — a forward convention, explicitly not a recovery. 2,426 such beliefs carry only 433 distinct timestamps, so 94% sit inside a tie where the only other durable column is a content-addressed id. Three placement rules were measured with one link of spread between them. That ordering is unreconstructible, and the module says so in those words rather than leaving it as a caveat. The convention buys determinism, not correctness about the past.

Why three buckets and never one percentage

Only one of the three is a defect anyone can fix. A single number lets the unreconstructible bucket mask a real key disagreement — which is precisely how the first reading of this data concluded that AC2a was recoverable. missing_other is the bucket a wrong key moves, and it is 3.

Verification

  • Full suite: 7,092 passed, 69 skipped, 71 xfailed.
  • Every rule mutation-verified to go red, each hitting the test that names it: dropping the synth exclusion, flipping the no-log sentinel last→first, keying the ULID ahead of created_at, taking the latest log row instead of the earliest, chaining across session boundaries.
  • Two of the tests are controls, not assertions about the fix. test_divergence_is_zero_when_the_writer_agrees keeps the bucket tests honest — a recompute producing nothing would file every shipped edge under some bucket and look like correct attribution. The empty-store case pins that zero edges is full reproduction, not 0%, so a fresh store doesn't read as broken.
  • Read-only, and opened read-only (fix(bench): two diagnostics open the live store read-write, so running them can flip a user's locks (#1314 sweep) #1328). 0.86 s on a 44,594-belief store.
  • Four atomic signed commits, rebased on main, discretion clean.

Follow-ups this deliberately does not do

  • The writer change. Moving session_predecessor_id off (created_at, rowid) onto the ratified key. Until that lands, AC4's divergence test measures the gap rather than drift — the ruling is explicit that funding the recompute without it buys a test red by construction.
  • The 546 fan-in-2 successors. A writer defect; AC4 should exclude them and assert the count is non-increasing rather than set a percentage tolerance, which would hide it.
  • AC2's contract wording — guarantee-with-tiebreak or observation-with-the-0.017%-number. That is a wording call, not code.

Summary by Sourcery

Add a read-only command to recompute the temporal spine from the ingest log, report divergence by cause, and document the gap in the changelog, backed by tests for recomputation and bucketed divergence attribution.

New Features:

  • Introduce aelf spine verify to recompute TEMPORAL_NEXT from the ingest log and report divergence by bucket on a live store.

Enhancements:

  • Factor log-based temporal spine recomputation and divergence reporting into a dedicated spine_recompute module.

Documentation:

  • Document the temporal spine recomputation, gap measurement, and non-heuristic rules in the v4 changelog.

Tests:

  • Add a test suite exercising spine recomputation rules, synth-log exclusion, no-log ordering convention, and divergence bucketing semantics, including control cases for zero divergence and empty stores.

@robotrocketscience robotrocketscience added the author-Kulili PR coordination mutex label Aug 4, 2026

@sourcery-ai sourcery-ai Bot 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.

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@robotrocketscience robotrocketscience added the attn:review Needs review (PR open, awaiting reviewer) label Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@robotrocketscience, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7516aaff-f5fb-4980-ba67-417f72b048ce

📥 Commits

Reviewing files that changed from the base of the PR and between f1fa6d1 and 94d0ddb.

📒 Files selected for processing (4)
  • CHANGELOG/v4.md
  • src/aelfrice/cli.py
  • src/aelfrice/spine_recompute.py
  • tests/test_spine_recompute_1283.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements a diagnostic command to recompute the temporal spine from the ingest log, report divergence by cause, and codifies the recomputation rules and verification tests without changing the writer.

Sequence diagram for aelf spine verify recomputation and divergence report

sequenceDiagram
    actor Operator
    participant CLI as _cmd_spine
    participant Verify as _cmd_spine_verify
    participant Store as MemoryStore
    participant Recompute as spine_divergence
    participant Edges as recompute_spine_edges

    Operator->>CLI: aelf spine verify
    CLI->>Verify: _cmd_spine_verify(out)
    Verify->>Store: MemoryStore(db_path, read_only=True)
    Verify->>Recompute: spine_divergence(Store)
    Recompute->>Edges: recompute_spine_edges(Store)
    Edges-->>Recompute: recomputed_edges, no_log_ids
    Recompute-->>Verify: SpineDivergence report
    Verify->>Store: close()
    Verify-->>Operator: print shipped / recomputed / reproduced and buckets
Loading

File-Level Changes

Change Details Files
Add aelf spine verify CLI subcommand to recompute the temporal spine and print a bucketed divergence report against the shipped TEMPORAL_NEXT edges.
  • Route spine verify action in _cmd_spine to a new _cmd_spine_verify helper and keep other actions using the existing store open path.
  • Open a read-only MemoryStore for verification, call spine_divergence, and print shipped/recomputed/reproduced counts plus three divergence buckets with explanatory text.
  • Extend the spine subcommand argparse choices and help string to document the new verify action and its read-only, diagnostic nature.
src/aelfrice/cli.py
Document the new temporal spine verification capability and its semantics in the v4 changelog.
  • Add a detailed v4 changelog entry describing the recompute key (created_at, ingest_log ULID), the gap vs drift distinction, and the three non-heuristic rules including synth log exclusion and no-log ordering convention.
  • Record the measured reproduction statistics and clarify that divergence is reported in three buckets rather than a single percentage.
CHANGELOG/v4.md
Introduce spine_recompute module that recomputes TEMPORAL_NEXT edges from ingest_log on the ratified durable key and classifies divergence into three buckets.
  • Define SYNTH_SOURCE_KIND and implement _log_sort_keys to map beliefs to their earliest qualifying ingest log ULID while excluding migration-synth rows by source_kind.
  • Implement recompute_spine_edges to build per-session chains ordered by (created_at, log ULID or sentinel, id), track beliefs with no log row, and return the recomputed edge set plus the no-log id set.
  • Implement SpineDivergence dataclass with summary counts, three miss buckets (touching no-log, fan-in, other), and a reproduced_share property that treats zero shipped edges as full reproduction.
  • Implement spine_divergence to compare shipped vs recomputed edges, compute per-successor fan-in, and assign misses into the three buckets, returning a SpineDivergence instance.
src/aelfrice/spine_recompute.py
Add targeted tests that pin the recomputation key rules, no-log ordering convention, determinism, and divergence bucket attribution for the temporal spine.
  • Set up a pinned test environment and a MemoryStore fixture to avoid touching any repo-local live store.
  • Add helpers to insert beliefs and custom ingest_log rows so ULIDs and source_kind can be controlled in tests.
  • Test ULID-based ordering within a shared created_at, earliest-log-row selection, created_at dominance over ULID, and session isolation in recomputed chains.
  • Test synth log exclusion via SYNTH_SOURCE_KIND and the resulting no-log behavior and downstream edge ordering.
  • Test the no-log convention (sort last by sentinel and then id), plus recompute determinism over repeated runs.
  • Add divergence tests: a control case where writer and recompute agree (zero divergence, 100% reproduction), a no-log miss classified into the missing_touching_no_log bucket, a fan-in miss classified into missing_fan_in, and an empty-store case that reports full reproduction rather than 0%.
  • Ensure tests reflect the contract that divergence is expected until the writer is changed and focus on bucket attribution rather than asserting zero drift.
tests/test_spine_recompute_1283.py

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

PR-size soft cap

This PR is over the advisory size threshold:

  • 938 changed lines (limit: 200)
  • 4 changed files (limit: 3)

Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated attn:merge-conflict cycles (see #602). When practical, split into smaller PRs that each touch a focused surface.

This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the size:override label and this comment will be removed on the next push.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Garsecg:2026-08-04T22:51:25Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Setr:2026-08-04T22:52:27Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Setr:2026-08-04T22:52:32Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

Review — the recompute is right and well-proven. One vacuous test fixed here; the branch needs a rebase.

I ran the gate measurement this PR was funded by, so I reviewed it against what that measurement actually established rather than against the issue body.

Pushed to this branch: 3257ef56 (signed, discretion clean).

Reproduced, not taken on trust

  • The headline divergence reproduces on a fresh .backup of the live store: buckets 2,100 / 546 / 3 identical to the digit. Shipped 41,971 vs your 41,929 and reproduced 39,322 vs your 39,280 — the store grew by exactly 42 edges and the reproduced count grew by exactly 42, so 93.68% → 93.69% is denominator drift, not a change.
  • n_recomputed = 41,424 = 42,008 session-scoped beliefs − 584 distinct sessions, confirmed by direct SQL. Fan-in histogram {1: 40,879, 2: 546} matches the gate measurement's 546 exactly. 5 session-scoped beliefs have a synth earliest log row, also matching.
  • Read-only verified at the byte level: sha256 of the store copy identical before and after both the module call and a full aelf spine verify.
  • All 5 claimed mutations go red, each hitting the test you name. Full suite green under a sandboxed HOME.
  • All 139,779 ingest_log.id values are 26 chars, so the ORDER BY id ASC lexicographic assumption is sound; source_kind is NOT NULL, so the != ? predicate has no NULL-exclusion trap.

Consistency with the operator's constraints, checked one by one: you ship item (1) only and explicitly defer the writer change and the AC4 fan-in assertion — you do not build past what was cleared. You refuse to key on migration-synth ULIDs via source_kind='legacy_unknown', per constraint (1). You bucket the 546 fan-in>1 misses separately rather than folding them into a percentage tolerance, per constraint (2) — which is the right call, because a tolerance would hide the writer defect that produces them.

Fixed here — nothing distinguished the ULID key from a plain belief-id sort

test_ulid_orders_beliefs_that_share_a_created_at's docstring says the tie is "broken by the log, not rowid". It only proves not rowid. Replacing the sort tuple's log component with a constant — i.e. sorting purely by (created_at, id) and never consulting the log — leaves 11/11 green. Every fixture picks ids whose alphabetical order coincides with their assigned ULID order (a<b<c with 01AAA<01BBB<01CCC; logged<orphan_a<orphan_b; real<synthetic), so a recompute that ignored the log entirely passes the suite.

The production code is correct — I confirmed with a probe where ULID order is the reverse of id order. But nothing pinned it, and this is the one property the whole PR rests on. It matters concretely: 96.5% of this store shares a created_at, so the belief id is doing the ordering wherever the ULID is ignored.

Added test_ulid_beats_belief_id_when_the_two_orders_disagree: aaa carries the last ULID and ccc the first, so the log says ccc-bbb-aaa while the id says aaa-bbb-ccc. The neutralising mutation now fails, and only on that test.

Also fixed here

  • The docstring overclaimed durability, against constraint (3). It read "the sort key is (created_at, log ULID or sentinel, id) and every component is durable". The ruling says: state it as a guarantee with a deterministic intra-millisecond tiebreak, or as an observed property with the cross-process exposure attached — "not wording that implies the stronger claim while resting on the weaker". There is no tiebreak here, and ulid.py's own docstring concedes monotonicity is per-process. Now stated as observed, with the measured 0.017% (10 groups / 20 rows, deciding zero links today) attached.
  • SYNTH_SOURCE_KIND redeclared "legacy_unknown", which models.INGEST_SOURCE_LEGACY_UNKNOWN already owns and which store.py and replay.py import. Rebound to the shared constant.

Left for you

  1. The synth-ULID exclusion is a verified no-op on this store, and nothing says so. With SYNTH_SOURCE_KIND monkeypatched to a never-matching value — so synth rows do supply keys — the report is byte-identical: reproduced 39,322, buckets 2,100/546/3, zero beliefs move. All 5 session-scoped synth-earliest beliefs also carry a non-synth row. The rule is right in principle and is a genuine forward safeguard against a store whose migration ran at a different time, but the CHANGELOG's "exact in both directions here" reads as a measured effect when the measured effect is zero. One clause fixes it. This is the same inert-lever shape as the k3 boost, and worth naming as such before it becomes folklore.
  2. _cmd_spine_verify has zero test coveragegrep -rn "_cmd_spine" tests/ returns nothing. 62 added CLI lines, including the read_only=True open (the whole fix(bench): two diagnostics open the live store read-write, so running them can flip a user's locks (#1314 sweep) #1328 rationale) and the gap-not-drift note you argue is load-bearing. A regression dropping read_only=True — which would re-run migrations and the feat: time-boxed locks — lock a belief for a bounded window, then let it age out #1314 lock sweep on a diagnostic path — ships green. I exercised it manually and it works; it just isn't guarded. Also _cmd_spine's own docstring still reads aelf spine {backfill,clear}.
  3. The branch is 9 behind github/main and will not fast-forward.

Housekeeping: running this suite currently rewrites the operator's real home (#1320, fix open at PR #1334). I ran everything behind a process-level HOME override — see ~/.claude/handoffs/ALL-2026-08-04-pytest-corrupts-real-dotdir.md.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Garsecg:2026-08-04T23:13:09Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Setr:2026-08-05T01:26:38Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Garsecg:2026-08-05T01:27:26Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Garsecg:2026-08-05T01:27:32Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:Idnn:2026-08-05T01:27:37Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Idnn:2026-08-05T01:27:43Z]

The ratified contract says edges are log-derived and the recompute key
is `(created_at, ingest_log ULID)`. The shipped writer orders by
`(created_at, rowid)`, and `rowid` is implicit — VACUUM may renumber it
— which is why the ratified key is the log's ULID rather than anything
read off the belief table. This is the recompute half only; the writer
is unchanged, so measuring against a live store reports a gap, not
drift. A zero here today would mean the recompute had been fitted to the
defect.

Three rules, none of them a heuristic.

Synth log rows are excluded by `source_kind = 'legacy_unknown'`, a
stated durable column that is exact in both directions on the
development store. Their ULID prefix is migration wall-clock and their
order is `beliefs.rowid` relabelled, so honouring them would launder
rowid order into the key the contract calls durable. Two heuristic
detectors were measured first and both failed: a prefix-vs-`ts`
disagreement threshold catches legitimately delayed derivation and
collapses reproduction to 34.75%, and excluding the largest ULID-prefix
date cluster would drop 51.8% of the log — a real backfill carrying
20,095 session-scoped beliefs.

A belief takes its earliest qualifying log row, because later rows are
corroborations and only the first records insertion.

Beliefs with no qualifying log row sort last within their `created_at`
group, then by id. This is a forward convention and explicitly not a
recovery: 2,426 such beliefs carry only 433 distinct timestamps, so 94%
sit inside a tie where the only other durable column is a
content-addressed id. Three placement rules were measured and produced
one link of spread between them. The convention buys determinism, not
correctness about the past.

Divergence is reported in three buckets rather than one percentage,
because only one of them is a defect anyone can fix and a single number
would let the unreconstructible bucket mask a real key disagreement.

On the development store: 41,929 shipped, 39,280 reproduced (93.68%),
misses 2,100 no-log / 546 fan-in / 3 other, in 0.86s. Those figures
reproduce the #1283 gate measurement and its correction exactly, from an
independent implementation.
Reports the recompute divergence by bucket. Opened read-only (#1328),
because a bare `MemoryStore` open runs migrations and the #1314 lock
sweep, and a diagnostic must not mutate its subject.

Exits 0 whatever the divergence. The writer has not moved onto the
ratified key yet, so a non-zero gap is the predicted state; making it an
error would turn every run red on a number the contract already
anticipates. The note in the output says which of the two it is, since
"93.68%" alone reads as drift.
Each rule has a test that goes red under a mutation of exactly that
rule, verified: dropping the synth exclusion, flipping the no-log
sentinel from last to first, keying on the ULID ahead of `created_at`,
taking the latest log row instead of the earliest, and chaining across
session boundaries.

Two of these are controls rather than assertions about the fix.
`test_divergence_is_zero_when_the_writer_agrees` keeps the bucket tests
honest — a recompute that produced nothing would file every shipped edge
under some bucket and look like correct attribution. And the empty-store
case pins that zero edges is full reproduction rather than 0%, so a
fresh store does not read as broken.

Nothing here asserts that divergence is zero on a real store. The writer
still keys on `rowid`, so a passing zero would mean the recompute had
been fitted to the defect rather than to the contract.
States the two things a reader would otherwise get wrong: that the
number is a gap against the ratified key rather than drift, because the
writer has not moved yet; and that the no-log bucket is a stated forward
convention rather than a recovery, because that ordering is
unreconstructible.
…claiming durability

Three things, all found by review.

1. Nothing distinguished the ratified sort key from a plain
   `(created_at, id)` sort. Replacing the log key with a constant left
   the whole file green: every fixture picked ids whose alphabetical
   order coincides with their assigned ULID order, so the two sorts
   produce identical chains. The new test opposes them — `aaa` carries
   the last ULID, `ccc` the first — so an id sort yields
   {(bbb,aaa),(ccc,bbb)} and the log yields {(bbb,ccc),(aaa,bbb)}.
   That mutation now fails, and only on the new test. This matters
   because 96.5% of the store shares a created_at, so the belief id is
   doing the ordering wherever the ULID is ignored.

2. The docstring said the sort key's components are all durable.
   Operator constraint (3) on #1283 forbids exactly that wording: state
   it as a guarantee with a deterministic intra-millisecond tiebreak, or
   as an observed property with the cross-process exposure attached.
   There is no tiebreak here, so it is now stated as observed, with the
   measured 0.017% (10 groups / 20 rows, deciding zero links today) and
   the pointer to ulid.py's own 'cross-process drift is possible but
   tolerated'.

3. SYNTH_SOURCE_KIND redeclared the literal 'legacy_unknown' that
   models.INGEST_SOURCE_LEGACY_UNKNOWN already owns and that store.py
   and replay.py import. Rebound to the shared constant.

Suite: 7116 passed, 69 skipped, 71 xfailed.
@robotrocketscience

Copy link
Copy Markdown
Owner Author

Rebased onto current main (was behind and could not fast-forward) — full suite 7,183 passed / 69 skipped / 71 xfailed under a sandboxed HOME, discretion clean, all five commits signed. No content change.

The operator ruling on #1283 landed after this PR was opened, and it ratifies the framing this PR already ships — recording that here so a reviewer does not re-open it:

  • AC2a is closed unrecoverable. No placement rule moves the number: 2,426 no-log beliefs carry only 433 distinct created_at values, ~94% inside a tie, and inside a tie the durable columns are the tied created_at and a content-addressed id. Three rules were measured, one link of spread, 93.68% every time. This PR's "forward convention, explicitly not a recovery" wording is the ratified position, not a hedge.
  • AC2's contract is observation-with-number, ceiling ~95.0% against the current writer rather than 98.70%. So the three-bucket report is right to refuse a single percentage, and the "gap against the contract, not drift" line is the ruling's own language.
  • Constraint (1) as amended keys on source_kind = 'legacy_unknown', never on a ULID-prefix date cluster — which is exactly what ships here, and the PR's measurement of why the two heuristic detectors fail is what closed that question.
  • The writer change is not funded. Until writer and recompute share a key, an AC4 divergence test measures this standing gap rather than drift; read the number as a gap meter, not a regression alarm.

One thing the ruling adds that is worth carrying in the docstring if anyone touches this again: the ULID prefix is backfill processing time (p50 14.1 d, max 58.5 d behind its own content), so it is a sound ordering key — 98.94% of adjacent pairs agree with content ts — but never a timestamp. This PR uses it only as a sort key, so it is correct as written; the risk is a future reader promoting it.

I authored this, so I cannot review it — author-Kulili is stamped and it is flagged attn:review.

The module docstring was corrected to state the ULID component as an
observed property rather than a guarantee, per the #1283 constraint
against wording that implies the stronger claim while resting on the
weaker. The test file's own header still read "every component
durable" — the same claim, in the file whose job is to pin it.
The docstring enumerated {backfill,clear} and predated the verify
action added alongside it, so the one action that opens read-only was
the one the docstring did not mention.
The command had no coverage at all: 62 lines including the
`read_only=True` open that is the entire reason a diagnostic may be
pointed at a live store (#1328). Dropping that keyword, or swapping the
open for `_open_store()`, shipped green.

Three arms, because one is not enough. A kwarg spy on the constructor
names the mutation at the call site but cannot see a refactor that
reaches past the patched name; a sha256 over the store file catches
that but reports a hash mismatch rather than a line; and an expired
user lock left locked pins the specific write a bare open performs —
the #1314 sweep — with a control asserting a read-write open does flip
it, so neither byte arm can pass vacuously.

The report-shape fixture gives all six printed counts distinct values
(10/7/4, buckets 3/2/1) so a bucket printed from the wrong field
fails; with equal counts that mutation is green.
`read_only=True` opens through SQLite's `mode=ro` URI, which refuses to
create the file. Every other `spine` action reaches the store through
`_open_store()`, so `verify` is the only one that meets a repo with
nothing built and it did so with an uncaught sqlite3.OperationalError:
`main()` wraps the dispatch in nothing, so the user got a raw traceback
indistinguishable from a corrupt store or a permissions fault.

A store that was never built is the expected state of a fresh repo, not
a diagnostic failure, so it reports one line and keeps the exit at 0
along with every other outcome of this command.
The entry read "exact in both directions here", which is true of the
column but reads as a measured effect. The measured effect is zero:
neutralising the exclusion leaves every bucket identical and the
recomputed edge set unchanged. 20,852 beliefs take a different sort key
and only 5 are session-scoped, each also carrying a non-synth row.

The rule is still right — it is a forward safeguard against a store
whose migration ran at a different time — but an inert lever that
nobody labels becomes folklore about why the number is what it is.
@robotrocketscience

Copy link
Copy Markdown
Owner Author

Review — every published number re-derives. One real defect fixed here, the CLI is now guarded, and the prior review's three items are closed.

I re-derived the load-bearing figures from a fresh .backup of the live store rather than reading them off the PR body, and audited the module for what two prior passes had not covered. Pushed five signed commits to this branch (3ed7bf7b..94d0ddb8), rebased onto your 51b4558d.

Store census on my copy: 44,652 active beliefs / 46,004 total / 139,893 log rows / 41,983 TEMPORAL_NEXT edges.

Reproduced independently

claim mine verdict
buckets 2,100 / 546 / 3 2,100 / 546 / 3 exact
shipped / recomputed / reproduced 41,983 / 41,436 / 39,334 = 93.69% +54 on all three vs the PR body — pure store growth, buckets frozen
2,426 no-log beliefs, 433 distinct created_at 2,426 / 433 exact
#263 window is 201 ms, 20,852 rows 2026-04-29T00:06:05.188Z .. .389Z, span 201 ms exactly exact
"exact in both directions" the non-trivial direction holds: 0 of 119,041 non-synth rows fall inside the window confirmed
read-only sha256 of the store file identical before and after a full aelf spine verify confirmed

One correction to the record, in your favour. I initially could not re-derive the CHANGELOG's "flags 30,738 rows"; a first pass over all log rows gives 86,858. That pass is measuring the wrong population. Applied where the rule actually operates — the earliest log row per belief, which is exactly what _log_sort_keys builds — it is 30,738 of 60,477, on the nose. Recording the site here because the number is right and the next reader will otherwise reach for the same wrong denominator I did.

Fixed here — aelf spine verify tracebacks on a repo that has no store yet

read_only=True opens through SQLite's mode=ro URI, which refuses to create the file. Every other spine action reaches the store through _open_store(), so verify is the only one that meets a fresh repo with nothing built — and main() wraps the dispatch in nothing, so it surfaced as:

  File ".../cli.py", line 6053, in _cmd_spine_verify
    store = MemoryStore(str(db_path()), read_only=True)
sqlite3.OperationalError: unable to open database file

Indistinguishable from a corrupt store or a permissions fault. It is the direct cost of the (correct) #1328 read-only open, and it is reachable in every git repo that has not been onboarded yet. A store that was never built is the expected state of a fresh repo, not a diagnostic failure, so it now reports one line and keeps the exit at 0 with every other outcome of this command. Three independent verifiers reproduced it before I touched it; a fourth reasonably argues severity is nearer a nit than a blocker, since spine is registered help=argparse.SUPPRESS and does not appear in aelf --help.

Fixed here — the CLI had no coverage at all, including read_only=True

grep -rn "_cmd_spine" tests/ returned nothing, so the keyword that makes it safe to point a diagnostic at a live store shipped unguarded. Three arms, because one is not enough:

  • a kwarg spy on the MemoryStore cli resolves names the mutation at the call site;
  • a sha256 over the store file catches the refactor the spy cannot see — swapping the open for _open_store() reaches past the patched name;
  • an expired user lock left locked pins the specific write a bare open performs, the feat: time-boxed locks — lock a belief for a bounded window, then let it age out #1314 sweep, with a control asserting a read-write open does flip it, so neither byte arm can pass vacuously.

Mutations I ran myself, each reverted after: dropping read_only=Truered ×3; read_only=False → red ×3; swapping to _open_store() → red ×3; deleting the gap-not-drift note → red ×1; deleting the new existence guard → red ×1. The report-shape fixture gives all six printed counts distinct values (10/7/4, buckets 3/2/1) so a bucket printed from the wrong field fails — with equal counts that mutation is green, which is worth stating because the first draft of the fixture had exactly that hole.

Closed the prior review's item 1 — the synth exclusion is inert here, and now says so

Re-ran the neutralisation myself: with the exclusion disabled so synth rows do supply keys, the SpineDivergence is equal, the recomputed edge sets differ by 0, and the no-log sets differ by 0. 20,852 beliefs take a different sort key under that arm, but only 5 are session-scoped and each of those also carries a non-synth row, so nothing moves. The rule is still right — a forward safeguard against a store whose migration ran at a different time — but "exact in both directions here" is a property of the column, not a measured effect, and the CHANGELOG now separates the two. Same inert-lever shape as the k3 boost, named before it becomes folklore about why the number is what it is.

Also fixed

  • The durability overclaim survived in the test file's own header. The module docstring was corrected to state the ULID component as an observed property; tests/test_spine_recompute_1283.py:3-4 still read "every component durable" — the same claim constraint (3) forbids, in the file whose job is to pin it.
  • _cmd_spine's docstring enumerated {backfill,clear}, so the one action that opens read-only was the one it did not mention.

Left for you — one reporting semantic I did not change unilaterally

reproduced_share returns 1.0 whenever n_shipped == 0. On an empty store that is right and the test says why. But aelf spine clear is a shipped subcommand four lines away, and after it the command prints:

shipped TEMPORAL_NEXT : 0
recomputed            : 41,436
reproduced            : 0 (100.00%)

100% reproduction of a spine that is entirely absent. That is the exact failure this PR is built to refuse — a single number covering for a state it does not describe. The fix is small (1.0 only when both sides are zero, else 0.0 or an n/a in the formatter) but it changes a published dataclass property and the shape of the printed line, so it is your call rather than mine. Two of three adversarial verifiers rejected it as a defect on the grounds that the convention is deliberate and documented; the documented case is both zero, and this is not that.

Verification

Not blocking on the reproduced_share item — labelling ready-to-merge.

@robotrocketscience robotrocketscience added the ready-to-merge Trigger merge-train: FF main to this PR's head label Aug 5, 2026
@github-actions
github-actions Bot merged commit 94d0ddb into main Aug 5, 2026
30 checks passed
@github-actions github-actions Bot removed the ready-to-merge Trigger merge-train: FF main to this PR's head label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

merge-train: merged 94d0ddbmain via FF push.

@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:Setr:2026-08-05T02:05:41Z]

robotrocketscience added a commit that referenced this pull request Aug 5, 2026
The memo still read 'Until that recompute exists', which went stale when
#1336 merged. It also omitted the rowid-exclusion rationale, which is the
load-bearing half of the 2026-08-01 ruling: the writer orders on
(created_at, rowid), that rowid is implicit under 'id TEXT PRIMARY KEY',
VACUUM may renumber it and it survives no rebuild — which is why the
ratified key is the log's ULID.

States what shipped rather than implying more: 'aelf spine verify' covers
TEMPORAL_NEXT only, reproduces ~93.7% against a ~95.0% ceiling under the
current writer, and reports the gap rather than closing it. Reaching the
98.70% structural ceiling needs the unfunded writer change, so points 3
and 4 stay edge-incomplete.

The pre-v2.0 'historical memo' header now notes that the ratified edge
contract it carries is current.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…edges

#1283 restated AC2 in two halves. The recompute half shipped in #1336;
this is the other one. Edges that are neither TEMPORAL_NEXT nor
DERIVED_FROM are a function of the belief set AND of detector
thresholds, so "edges are recomputable" holds for them only if those
thresholds are pinned and versioned. They were bare module constants
with no guard.

detector_thresholds records 22 constants across 8 modules behind
DETECTOR_THRESHOLDS_VERSION: the relationship_detector cutoffs, caps
and vocabularies, the contradiction precedence ladder, the
triple_extractor phrase-to-edge-type patterns, the value_compare slot
gate, and the constants on the two paths that decide which phantoms
reach the RELATES_TO writer.

The module holds hand-written literals and imports nothing from
aelfrice. That is deliberate: importing the constants it describes
would make it tautological in exactly the way the tests it replaces
were, and would drag store/bm25 into the import graph of any reader.
Scalars pin as literals so a reviewer can check them by eye;
collections pin as a digest of a canonical form that includes regex
flags, since dropping re.IGNORECASE changes which triples match
without changing any pattern text.

Entries were checked for reachability rather than assumed. The wonder
bake-off constants are NOT pinned -- the package docstring states the
strategies are research-only and their sole importer builds against an
in-memory store, so they decide no edge a user holds. The value_compare
entries are pinned but labelled dormant: nothing shipped passes
use_value_comparison=True. Three upstream suppliers the call-site sweep
cannot see (bm25._TOKEN_PATTERN, models.ANCHOR_TEXT_MAX_LEN,
wonder_consolidation._TOKENIZER_DROP) are pinned by hand, and the two
suppliers left unpinned are named so their absence is a decision.

Forward-only. The edges table has no version and no created_at, so
this does not make a historical edge attributable to the thresholds
that produced it; adding those columns is the migration that bricked
stores in #1161, and historical reproduction stays out of scope.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…usion

`gates` is the manifest's substance -- "a constant that cannot answer
this does not belong here" -- and nothing tests these strings, so five
wrong ones shipped. All five were reproduced against live source before
being rewritten.

- SUPERSEDES_WEIGHT claimed "propagation arithmetic". There is none:
  Edge.weight is read by a BFS sort key, a clustering floor and
  persistence. Propagation is EDGE_VALENCE, keyed on edge TYPE, which
  this constant does not touch. The real effect is sharper -- at 1.0 it
  clears DEFAULT_CLUSTER_EDGE_FLOOR (0.4), and any value below that
  silently drops every SUPERSEDES edge out of candidate clustering.
- UNCERTAINTY_THRESHOLD claimed to filter the anchor tuple. It does not:
  `anchors` is built from the unfiltered `known_beliefs`. It selects
  high_uncertainty_beliefs, which decides whether an
  uncertainty_deep_dive axis is emitted -- still a real path to
  RELATES_TO, but not the stated one.
- QUANT_AXIS said the score is half the axis distance. It is a quarter:
  q_term halves the distance and the score halves it again. `always` vs
  `sometimes` is 1.0 apart and scores 0.25, so it lands as
  POTENTIALLY_STALE rather than CONTRADICTS -- the wrong side of the very
  split DEFAULT_CONFIDENCE_MIN exists to record. Sizing an edit with the
  old prose picks the wrong value.
- DEFAULT_JACCARD_MIN claimed lowering it "can only add edges". The
  candidate pool is monotonic; the written set is not, because
  DEFAULT_MAX_EDGES_PER_BELIEF is spent in sorted pair order, so a
  newly-admitted pair can evict a previously written one.
- _PATTERNS omitted TEMPORAL_NEXT from edge_types while four of its 25
  patterns mint exactly that type (`follows`, `comes after`, `is after`,
  `succeeds`).

That last one has a consequence outside the manifest, so the exclusion
entry now carries it: EXCLUDED_WRITERS said temporal_spine "writes
TEMPORAL_NEXT only", which reads as the spine accounting for the whole
TEMPORAL_NEXT population. It does not -- triple_extractor is a second,
prose-driven producer the #1336 spine recompute does not cover.

Also records two limits the manifest was silent on. `--axes-budget`
(default 24) caps the anchor tuple and so how many RELATES_TO edges each
persisted phantom writes -- a bigger lever than several pinned constants,
but a signature default rather than a module constant, out of reach of
the (module, name) scheme for the same reason the inline weights are.
And belief ARRIVAL ORDER is a third input beside the belief set and these
thresholds: the per-belief cap is spent on whichever pairs arrived first,
so full-store and incremental runs can disagree on identical beliefs, and
re-deriving edges from beliefs plus this manifest gives a false mismatch
on any incrementally built store -- which is every real one.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…edges

#1283 restated AC2 in two halves. The recompute half shipped in #1336;
this is the other one. Edges that are neither TEMPORAL_NEXT nor
DERIVED_FROM are a function of the belief set AND of detector
thresholds, so "edges are recomputable" holds for them only if those
thresholds are pinned and versioned. They were bare module constants
with no guard.

detector_thresholds records 22 constants across 8 modules behind
DETECTOR_THRESHOLDS_VERSION: the relationship_detector cutoffs, caps
and vocabularies, the contradiction precedence ladder, the
triple_extractor phrase-to-edge-type patterns, the value_compare slot
gate, and the constants on the two paths that decide which phantoms
reach the RELATES_TO writer.

The module holds hand-written literals and imports nothing from
aelfrice. That is deliberate: importing the constants it describes
would make it tautological in exactly the way the tests it replaces
were, and would drag store/bm25 into the import graph of any reader.
Scalars pin as literals so a reviewer can check them by eye;
collections pin as a digest of a canonical form that includes regex
flags, since dropping re.IGNORECASE changes which triples match
without changing any pattern text.

Entries were checked for reachability rather than assumed. The wonder
bake-off constants are NOT pinned -- the package docstring states the
strategies are research-only and their sole importer builds against an
in-memory store, so they decide no edge a user holds. The value_compare
entries are pinned but labelled dormant: nothing shipped passes
use_value_comparison=True. Three upstream suppliers the call-site sweep
cannot see (bm25._TOKEN_PATTERN, models.ANCHOR_TEXT_MAX_LEN,
wonder_consolidation._TOKENIZER_DROP) are pinned by hand, and the two
suppliers left unpinned are named so their absence is a decision.

Forward-only. The edges table has no version and no created_at, so
this does not make a historical edge attributable to the thresholds
that produced it; adding those columns is the migration that bricked
stores in #1161, and historical reproduction stays out of scope.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…usion

`gates` is the manifest's substance -- "a constant that cannot answer
this does not belong here" -- and nothing tests these strings, so five
wrong ones shipped. All five were reproduced against live source before
being rewritten.

- SUPERSEDES_WEIGHT claimed "propagation arithmetic". There is none:
  Edge.weight is read by a BFS sort key, a clustering floor and
  persistence. Propagation is EDGE_VALENCE, keyed on edge TYPE, which
  this constant does not touch. The real effect is sharper -- at 1.0 it
  clears DEFAULT_CLUSTER_EDGE_FLOOR (0.4), and any value below that
  silently drops every SUPERSEDES edge out of candidate clustering.
- UNCERTAINTY_THRESHOLD claimed to filter the anchor tuple. It does not:
  `anchors` is built from the unfiltered `known_beliefs`. It selects
  high_uncertainty_beliefs, which decides whether an
  uncertainty_deep_dive axis is emitted -- still a real path to
  RELATES_TO, but not the stated one.
- QUANT_AXIS said the score is half the axis distance. It is a quarter:
  q_term halves the distance and the score halves it again. `always` vs
  `sometimes` is 1.0 apart and scores 0.25, so it lands as
  POTENTIALLY_STALE rather than CONTRADICTS -- the wrong side of the very
  split DEFAULT_CONFIDENCE_MIN exists to record. Sizing an edit with the
  old prose picks the wrong value.
- DEFAULT_JACCARD_MIN claimed lowering it "can only add edges". The
  candidate pool is monotonic; the written set is not, because
  DEFAULT_MAX_EDGES_PER_BELIEF is spent in sorted pair order, so a
  newly-admitted pair can evict a previously written one.
- _PATTERNS omitted TEMPORAL_NEXT from edge_types while four of its 25
  patterns mint exactly that type (`follows`, `comes after`, `is after`,
  `succeeds`).

That last one has a consequence outside the manifest, so the exclusion
entry now carries it: EXCLUDED_WRITERS said temporal_spine "writes
TEMPORAL_NEXT only", which reads as the spine accounting for the whole
TEMPORAL_NEXT population. It does not -- triple_extractor is a second,
prose-driven producer the #1336 spine recompute does not cover.

Also records two limits the manifest was silent on. `--axes-budget`
(default 24) caps the anchor tuple and so how many RELATES_TO edges each
persisted phantom writes -- a bigger lever than several pinned constants,
but a signature default rather than a module constant, out of reach of
the (module, name) scheme for the same reason the inline weights are.
And belief ARRIVAL ORDER is a third input beside the belief set and these
thresholds: the per-belief cap is spent on whichever pairs arrived first,
so full-store and incremental runs can disagree on identical beliefs, and
re-deriving edges from beliefs plus this manifest gives a false mismatch
on any incrementally built store -- which is every real one.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
The entry read "published as `93.68%` (39,335 / 41,984) and is now `94.86%`",
but 39,335 / 41,984 is 93.69%. The 93.68% belongs to #1336's measurement,
39,280 / 41,929 — a snapshot 55 shipped edges smaller — which the unchanged
#1283 entry three lines below still reports as such.

So the pair presented as a pure denominator correction spanned two store states,
and about 0.01pp of the 1.18pp move was snapshot drift rather than the
correction. The module's own docstring already said 93.69%, so the branch
shipped both answers.

Both figures now come from one `spine_divergence()` call on one store, and the
#1336 figure is named separately rather than standing in as the before.

The design memo's "98.70% structural ceiling" is corrected the same way: it is
defined by the fan-in misses this change removes from the denominator, so it is
not the ceiling for 94.86% and the memo now says so rather than inviting the
subtraction it forbids two sentences earlier.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
The entry read "published as `93.68%` (39,335 / 41,984) and is now `94.86%`",
but 39,335 / 41,984 is 93.69%. The 93.68% belongs to #1336's measurement,
39,280 / 41,929 — a snapshot 55 shipped edges smaller — which the unchanged
#1283 entry three lines below still reports as such.

So the pair presented as a pure denominator correction spanned two store states,
and about 0.01pp of the 1.18pp move was snapshot drift rather than the
correction. The module's own docstring already said 93.69%, so the branch
shipped both answers.

Both figures now come from one `spine_divergence()` call on one store, and the
#1336 figure is named separately rather than standing in as the before.

The design memo's "98.70% structural ceiling" is corrected the same way: it is
defined by the fan-in misses this change removes from the denominator, so it is
not the ceiling for 94.86% and the memo now says so rather than inviting the
subtraction it forbids two sentences earlier.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…edges

#1283 restated AC2 in two halves. The recompute half shipped in #1336;
this is the other one. Edges that are neither TEMPORAL_NEXT nor
DERIVED_FROM are a function of the belief set AND of detector
thresholds, so "edges are recomputable" holds for them only if those
thresholds are pinned and versioned. They were bare module constants
with no guard.

detector_thresholds records 22 constants across 8 modules behind
DETECTOR_THRESHOLDS_VERSION: the relationship_detector cutoffs, caps
and vocabularies, the contradiction precedence ladder, the
triple_extractor phrase-to-edge-type patterns, the value_compare slot
gate, and the constants on the two paths that decide which phantoms
reach the RELATES_TO writer.

The module holds hand-written literals and imports nothing from
aelfrice. That is deliberate: importing the constants it describes
would make it tautological in exactly the way the tests it replaces
were, and would drag store/bm25 into the import graph of any reader.
Scalars pin as literals so a reviewer can check them by eye;
collections pin as a digest of a canonical form that includes regex
flags, since dropping re.IGNORECASE changes which triples match
without changing any pattern text.

Entries were checked for reachability rather than assumed. The wonder
bake-off constants are NOT pinned -- the package docstring states the
strategies are research-only and their sole importer builds against an
in-memory store, so they decide no edge a user holds. The value_compare
entries are pinned but labelled dormant: nothing shipped passes
use_value_comparison=True. Three upstream suppliers the call-site sweep
cannot see (bm25._TOKEN_PATTERN, models.ANCHOR_TEXT_MAX_LEN,
wonder_consolidation._TOKENIZER_DROP) are pinned by hand, and the two
suppliers left unpinned are named so their absence is a decision.

Forward-only. The edges table has no version and no created_at, so
this does not make a historical edge attributable to the thresholds
that produced it; adding those columns is the migration that bricked
stores in #1161, and historical reproduction stays out of scope.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…usion

`gates` is the manifest's substance -- "a constant that cannot answer
this does not belong here" -- and nothing tests these strings, so five
wrong ones shipped. All five were reproduced against live source before
being rewritten.

- SUPERSEDES_WEIGHT claimed "propagation arithmetic". There is none:
  Edge.weight is read by a BFS sort key, a clustering floor and
  persistence. Propagation is EDGE_VALENCE, keyed on edge TYPE, which
  this constant does not touch. The real effect is sharper -- at 1.0 it
  clears DEFAULT_CLUSTER_EDGE_FLOOR (0.4), and any value below that
  silently drops every SUPERSEDES edge out of candidate clustering.
- UNCERTAINTY_THRESHOLD claimed to filter the anchor tuple. It does not:
  `anchors` is built from the unfiltered `known_beliefs`. It selects
  high_uncertainty_beliefs, which decides whether an
  uncertainty_deep_dive axis is emitted -- still a real path to
  RELATES_TO, but not the stated one.
- QUANT_AXIS said the score is half the axis distance. It is a quarter:
  q_term halves the distance and the score halves it again. `always` vs
  `sometimes` is 1.0 apart and scores 0.25, so it lands as
  POTENTIALLY_STALE rather than CONTRADICTS -- the wrong side of the very
  split DEFAULT_CONFIDENCE_MIN exists to record. Sizing an edit with the
  old prose picks the wrong value.
- DEFAULT_JACCARD_MIN claimed lowering it "can only add edges". The
  candidate pool is monotonic; the written set is not, because
  DEFAULT_MAX_EDGES_PER_BELIEF is spent in sorted pair order, so a
  newly-admitted pair can evict a previously written one.
- _PATTERNS omitted TEMPORAL_NEXT from edge_types while four of its 25
  patterns mint exactly that type (`follows`, `comes after`, `is after`,
  `succeeds`).

That last one has a consequence outside the manifest, so the exclusion
entry now carries it: EXCLUDED_WRITERS said temporal_spine "writes
TEMPORAL_NEXT only", which reads as the spine accounting for the whole
TEMPORAL_NEXT population. It does not -- triple_extractor is a second,
prose-driven producer the #1336 spine recompute does not cover.

Also records two limits the manifest was silent on. `--axes-budget`
(default 24) caps the anchor tuple and so how many RELATES_TO edges each
persisted phantom writes -- a bigger lever than several pinned constants,
but a signature default rather than a module constant, out of reach of
the (module, name) scheme for the same reason the inline weights are.
And belief ARRIVAL ORDER is a third input beside the belief set and these
thresholds: the per-belief cap is spent on whichever pairs arrived first,
so full-store and incremental runs can disagree on identical beliefs, and
re-deriving edges from beliefs plus this manifest gives a false mismatch
on any incrementally built store -- which is every real one.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…edges

#1283 restated AC2 in two halves. The recompute half shipped in #1336;
this is the other one. Edges that are neither TEMPORAL_NEXT nor
DERIVED_FROM are a function of the belief set AND of detector
thresholds, so "edges are recomputable" holds for them only if those
thresholds are pinned and versioned. They were bare module constants
with no guard.

detector_thresholds records 22 constants across 8 modules behind
DETECTOR_THRESHOLDS_VERSION: the relationship_detector cutoffs, caps
and vocabularies, the contradiction precedence ladder, the
triple_extractor phrase-to-edge-type patterns, the value_compare slot
gate, and the constants on the two paths that decide which phantoms
reach the RELATES_TO writer.

The module holds hand-written literals and imports nothing from
aelfrice. That is deliberate: importing the constants it describes
would make it tautological in exactly the way the tests it replaces
were, and would drag store/bm25 into the import graph of any reader.
Scalars pin as literals so a reviewer can check them by eye;
collections pin as a digest of a canonical form that includes regex
flags, since dropping re.IGNORECASE changes which triples match
without changing any pattern text.

Entries were checked for reachability rather than assumed. The wonder
bake-off constants are NOT pinned -- the package docstring states the
strategies are research-only and their sole importer builds against an
in-memory store, so they decide no edge a user holds. The value_compare
entries are pinned but labelled dormant: nothing shipped passes
use_value_comparison=True. Three upstream suppliers the call-site sweep
cannot see (bm25._TOKEN_PATTERN, models.ANCHOR_TEXT_MAX_LEN,
wonder_consolidation._TOKENIZER_DROP) are pinned by hand, and the two
suppliers left unpinned are named so their absence is a decision.

Forward-only. The edges table has no version and no created_at, so
this does not make a historical edge attributable to the thresholds
that produced it; adding those columns is the migration that bricked
stores in #1161, and historical reproduction stays out of scope.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…edges

#1283 restated AC2 in two halves. The recompute half shipped in #1336;
this is the other one. Edges that are neither TEMPORAL_NEXT nor
DERIVED_FROM are a function of the belief set AND of detector
thresholds, so "edges are recomputable" holds for them only if those
thresholds are pinned and versioned. They were bare module constants
with no guard.

detector_thresholds records 22 constants across 8 modules behind
DETECTOR_THRESHOLDS_VERSION: the relationship_detector cutoffs, caps
and vocabularies, the contradiction precedence ladder, the
triple_extractor phrase-to-edge-type patterns, the value_compare slot
gate, and the constants on the two paths that decide which phantoms
reach the RELATES_TO writer.

The module holds hand-written literals and imports nothing from
aelfrice. That is deliberate: importing the constants it describes
would make it tautological in exactly the way the tests it replaces
were, and would drag store/bm25 into the import graph of any reader.
Scalars pin as literals so a reviewer can check them by eye;
collections pin as a digest of a canonical form that includes regex
flags, since dropping re.IGNORECASE changes which triples match
without changing any pattern text.

Entries were checked for reachability rather than assumed. The wonder
bake-off constants are NOT pinned -- the package docstring states the
strategies are research-only and their sole importer builds against an
in-memory store, so they decide no edge a user holds. The value_compare
entries are pinned but labelled dormant: nothing shipped passes
use_value_comparison=True. Three upstream suppliers the call-site sweep
cannot see (bm25._TOKEN_PATTERN, models.ANCHOR_TEXT_MAX_LEN,
wonder_consolidation._TOKENIZER_DROP) are pinned by hand, and the two
suppliers left unpinned are named so their absence is a decision.

Forward-only. The edges table has no version and no created_at, so
this does not make a historical edge attributable to the thresholds
that produced it; adding those columns is the migration that bricked
stores in #1161, and historical reproduction stays out of scope.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
…usion

`gates` is the manifest's substance -- "a constant that cannot answer
this does not belong here" -- and nothing tests these strings, so five
wrong ones shipped. All five were reproduced against live source before
being rewritten.

- SUPERSEDES_WEIGHT claimed "propagation arithmetic". There is none:
  Edge.weight is read by a BFS sort key, a clustering floor and
  persistence. Propagation is EDGE_VALENCE, keyed on edge TYPE, which
  this constant does not touch. The real effect is sharper -- at 1.0 it
  clears DEFAULT_CLUSTER_EDGE_FLOOR (0.4), and any value below that
  silently drops every SUPERSEDES edge out of candidate clustering.
- UNCERTAINTY_THRESHOLD claimed to filter the anchor tuple. It does not:
  `anchors` is built from the unfiltered `known_beliefs`. It selects
  high_uncertainty_beliefs, which decides whether an
  uncertainty_deep_dive axis is emitted -- still a real path to
  RELATES_TO, but not the stated one.
- QUANT_AXIS said the score is half the axis distance. It is a quarter:
  q_term halves the distance and the score halves it again. `always` vs
  `sometimes` is 1.0 apart and scores 0.25, so it lands as
  POTENTIALLY_STALE rather than CONTRADICTS -- the wrong side of the very
  split DEFAULT_CONFIDENCE_MIN exists to record. Sizing an edit with the
  old prose picks the wrong value.
- DEFAULT_JACCARD_MIN claimed lowering it "can only add edges". The
  candidate pool is monotonic; the written set is not, because
  DEFAULT_MAX_EDGES_PER_BELIEF is spent in sorted pair order, so a
  newly-admitted pair can evict a previously written one.
- _PATTERNS omitted TEMPORAL_NEXT from edge_types while four of its 25
  patterns mint exactly that type (`follows`, `comes after`, `is after`,
  `succeeds`).

That last one has a consequence outside the manifest, so the exclusion
entry now carries it: EXCLUDED_WRITERS said temporal_spine "writes
TEMPORAL_NEXT only", which reads as the spine accounting for the whole
TEMPORAL_NEXT population. It does not -- triple_extractor is a second,
prose-driven producer the #1336 spine recompute does not cover.

Also records two limits the manifest was silent on. `--axes-budget`
(default 24) caps the anchor tuple and so how many RELATES_TO edges each
persisted phantom writes -- a bigger lever than several pinned constants,
but a signature default rather than a module constant, out of reach of
the (module, name) scheme for the same reason the inline weights are.
And belief ARRIVAL ORDER is a third input beside the belief set and these
thresholds: the per-belief cap is spent on whichever pairs arrived first,
so full-store and incremental runs can disagree on identical beliefs, and
re-deriving edges from beliefs plus this manifest gives a false mismatch
on any incrementally built store -- which is every real one.
robotrocketscience added a commit that referenced this pull request Aug 6, 2026
The entry read "published as `93.68%` (39,335 / 41,984) and is now `94.86%`",
but 39,335 / 41,984 is 93.69%. The 93.68% belongs to #1336's measurement,
39,280 / 41,929 — a snapshot 55 shipped edges smaller — which the unchanged

So the pair presented as a pure denominator correction spanned two store states,
and about 0.01pp of the 1.18pp move was snapshot drift rather than the
correction. The module's own docstring already said 93.69%, so the branch
shipped both answers.

Both figures now come from one `spine_divergence()` call on one store, and the

The design memo's "98.70% structural ceiling" is corrected the same way: it is
defined by the fan-in misses this change removes from the denominator, so it is
not the ceiling for 94.86% and the memo now says so rather than inviting the
subtraction it forbids two sentences earlier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

attn:review Needs review (PR open, awaiting reviewer) author-Kulili PR coordination mutex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant