Skip to content

fix(cycle): case-insensitive page-atom identity hash - #5031

Open
Masashi-Ono0611 wants to merge 2 commits into
garrytan:masterfrom
Masashi-Ono0611:fix/atom-title-case-insensitive-identity
Open

fix(cycle): case-insensitive page-atom identity hash#5031
Masashi-Ono0611 wants to merge 2 commits into
garrytan:masterfrom
Masashi-Ono0611:fix/atom-title-case-insensitive-identity

Conversation

@Masashi-Ono0611

Copy link
Copy Markdown
Contributor

What

atomSlug's identity hash for page-derived atoms now hashes title.toLowerCase() instead of the raw title, and resolvePageAtomSlug gained a third fallback so an atom already minted under the old (un-lowercased) hash is adopted on re-extraction rather than duplicated.

Fixes #5030.

Why

extract_atoms mints a new atom identity — a duplicate — when the model's title for the same underlying claim differs only in letter case between extractions, because the identity hash for page-derived atoms was computed over the raw title. Full report, reproduction, and the maintainer's separate confirmation this is a distinct defect from the atom-retirement product question in #4908: #5030.

How

  • atomSlug() (src/core/cycle/extract-atoms.ts): page-derived branch (sourcePageSlug !== undefined) now hashes title.toLowerCase(). Transcript atoms and atomSlugStem (the human-readable slug prefix) are unchanged.
  • resolvePageAtomSlug(): added a third fallback after the exact new-shape and exact pre-fix(extract-atoms): prevent deterministic slug collisions from corrupting provenance #4733 legacy-shape checks both miss — search this source page's live atoms (frontmatter->>'source_slug' = sourcePageSlug, exact match only) and, if exactly one has a matching lowercased title, adopt its slug. Ambiguous (2+) matches fall through to minting a fresh slug rather than guessing.
  • Title comparison happens in application code with JavaScript .toLowerCase(), not SQL LOWER() — the two disagree for some non-ASCII titles (e.g. Greek "ΟΣ" lowercases to "οσ" in PostgreSQL/PGLite but "ος" in JS), which would silently miss an adoption if pushed into SQL.
  • Deliberately narrower than the legacy-slug path's isCompatibleAtomBinding rule: this fallback finds candidates by title search rather than a deterministic address, so it must not also adopt unbound (pre-binding-era) atoms from elsewhere in the source — only exact same-page bindings qualify. (An earlier version of this PR used isCompatibleAtomBinding directly and was flagged in review for exactly this over-reach; narrowed before submission.)
  • No deletion, merge, or retagging of any existing duplicate pair already live in a brain. In the common case (exactly one pre-existing case-variant atom on the page, and neither exact-slug check above finds it), re-extraction now converges onto it instead of minting a new one. If both exact-slug checks miss and the same-page title search finds 2+ case-variant atoms, the fallback deliberately does not guess between them and mints a fresh slug — existing duplicates are not merged or removed.
  • scripts/module-size-limits.tsv: ceiling raised for extract-atoms.ts (+77 lines) with a note.

Discrimination test

bash scripts/check-test-discriminates.sh test/extract-atoms-provenance-links.test.ts src/core/cycle/extract-atoms.ts:

Discrimination test: reverted src/core/cycle/extract-atoms.ts to merge-base, ran test/extract-atoms-provenance-links.test.ts → 12 pass / 11 fail. Restored → all pass.

Seven new test scenarios (in describe('case-insensitive atom identity (#4908)', ...)): core case-variance dedup, old-hash adoption with identical title case, old-hash adoption with different title case, ambiguous-candidates fallback (no guessing), distinct titles sharing a truncated 60-char stem staying on distinct slugs, an unbound atom with a matching title NOT being adopted, and a non-ASCII (Greek) case-variance scenario exercising the JS-vs-SQL lowercasing difference. Three pre-existing tests in the same file's #4733 describe block computed their expected slug inline with the pre-fix (non-lowercased) hash formula and were updated to track the new formula; all 23 tests in the file pass.

Scope check

  • bun run typecheck: clean.
  • bash scripts/check-module-size.sh: OK.
  • No new CLI surface, no new config, no new doctor check, no migration.

Masashi-Ono0611 and others added 2 commits September 11, 2026 03:20
extract_atoms minted a duplicate page-derived atom whenever a
re-extraction's model-returned title differed from a prior extraction
only in letter case: atomSlug hashed the raw title, so
"12-month price momentum..." and "12-Month Price Momentum..." hashed
to different slugs and both atoms lived simultaneously with nothing
to reconcile them (confirmed on upstream master, reported in gbrain
issue garrytan#4908).

Two-part fix:
- atomSlug now lowercases the title (plain .toLowerCase(), not full
  Unicode case-folding) before hashing it into the page-derived
  identity slug. atomSlugStem and the transcript-atom branch are
  untouched.
- resolvePageAtomSlug gains a third fallback: when neither the exact
  new-shape slug nor the exact pre-garrytan#4733 legacy-shape slug exist, it
  searches this page's live atoms case-insensitively by title and
  adopts the sole compatible match (via the existing
  isCompatibleAtomBinding predicate) — an atom minted under the OLD
  un-lowercased hash needs this to be found by a post-fix
  re-extraction, or the hash change alone would mint yet another
  duplicate. Two or more compatible matches is treated as ambiguous:
  no "pick the newest"/"pick the first" heuristic, fall through and
  mint a fresh slug, same as the existing no-compatible-legacy-row
  path already does.

No deletion, merge, or migration of any pre-existing duplicate pair —
this only prevents NEW duplicates during ordinary re-extraction by
reusing an existing atom's identity slug.

Extends test/extract-atoms-provenance-links.test.ts with the core
case-only-variance regression, both OLD-hash upgrade-adoption cases
(identical and different re-extraction case), the ambiguous
multi-candidate fallback, and a shared-truncated-stem non-collision
check; updates three pre-existing garrytan#4733 tests whose manually mirrored
hash formula needed the same .toLowerCase() to keep matching
production behavior. Adds a module-size-limits.tsv ceiling for
extract-atoms.ts, now over the unlisted 1500-line cap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vHSUY8QP4VdxoRo741o9G
…are titles in JS not SQL

Review round found two issues in the initial garrytan#4908 fix:
- maintainer-lens: the fallback reused isCompatibleAtomBinding wholesale,
  which also treats unbound (pre-binding-era) atoms as adoptable. Unlike
  the legacy-slug path (which computes an exact deterministic address
  first), this fallback finds candidates by title search, so adopting an
  unbound atom from elsewhere in the source is a broader claim than the
  garrytan#4733 precedent establishes. Narrowed to require an explicit same-page
  source_slug binding.
- code-quality: SQL LOWER() and JavaScript .toLowerCase() disagree for
  some non-ASCII titles (reproduced with Greek "ΟΣ"/"ος"). Moved the
  title comparison into application code.

Added regression tests for both (unbound-atom non-adoption, Greek
case variance) confirmed to fail against the prior implementation and
pass against this one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vHSUY8QP4VdxoRo741o9G
@Masashi-Ono0611
Masashi-Ono0611 marked this pull request as ready for review September 11, 2026 00:59
Masashi-Ono0611 added a commit to Masashi-Ono0611/gbrain that referenced this pull request Sep 11, 2026
…ytan#5031 — Ready, CI green)

atomSlug's identity hash for page-derived atoms now hashes title.toLowerCase(),
and resolvePageAtomSlug gained a same-page adoption fallback so an atom
minted under the old (un-lowercased) hash is found and reused instead of
duplicated. Fixes gbrain#5030.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vHSUY8QP4VdxoRo741o9G
@Masashi-Ono0611

Copy link
Copy Markdown
Contributor Author

Applied to my live install (production brain, tier-configured, real data — not a test fixture).

Deployment method: exported this patch as a content-addressed overlay from a local patch-stack branch built on this PR's exact head (b7d51e7d1), transferred it, and applied it. Post-apply verification (gbrain-patches-verify.py) confirms the live tree is byte-for-byte identical to the exported tip across all 53 tracked files (SHA-256 per file):

mode: APPLIED — exact final-tree verification
base=43597b19e50a3abf56409337f248f7966860293c tip=dbc369a018b295916afdab31e10e7b13e1c81095
commits=15 files=53
RESULT: exact match — live tree equals the exported patch-stack tip

Post-deploy health check (gbrain doctor --fast, $0, no LLM calls):

Overall health score: 95/100. All checks OK (some warnings).

(The one WARN is connection: Skipping DB checks (--fast mode), expected for --fast and unrelated to this change.)

Smoke test:

gbrain 0.48.5.0
[conversation-parser] 19 built-in patterns

I did not trigger a live extract_atoms re-extraction against production data for this comment (that requires an LLM call and I didn't want to spend one just for this verification round) — the behavioral evidence is the unit test suite in this PR (23 tests, including the 7 scenarios specific to this fix, all passing against the exact code now deployed here) plus the discrimination test showing the fix is necessary (12 pass / 11 fail without it).

Masashi-Ono0611 added a commit to Masashi-Ono0611/gbrain that referenced this pull request Sep 11, 2026
…ytan#5031 — Ready, CI green)

atomSlug's identity hash for page-derived atoms now hashes title.toLowerCase(),
and resolvePageAtomSlug gained a same-page adoption fallback so an atom
minted under the old (un-lowercased) hash is found and reused instead of
duplicated. Fixes gbrain#5030.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vHSUY8QP4VdxoRo741o9G
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extract_atoms creates duplicate page atoms when generated titles differ only in case

1 participant