Skip to content

docs(guide/05): say why superseded documents are kept, and clear three stale locator examples - #26

Merged
ancsingabor merged 2 commits into
mainfrom
docs/guide-05-review
Sep 28, 2026
Merged

ancsingabor merged 2 commits into
mainfrom
docs/guide-05-review

Conversation

@ancsingabor

Copy link
Copy Markdown
Owner

Review of docs/guide/05-data-model.md. Independent of #25 — branched from main, no overlapping files, so the two can merge in either order.

1. The rule without the reason

Superseded documents are kept, not deleted. Every read must filter documents.is_current = true.

That gives a reader a cost and no benefit. The benefit was two screens down in the choices table. Someone holding the rule without the reason eventually deletes the rows to save the duplication — and silently breaks every published citation. The reason now rides with the rule in both places.

2. summa:I.q2.a3 as a unit locator — three more sightings

PR #24 fixed architecture.md and amended ADR-002. Grepping only docs/ was my mistake. It also lives in:

Where Why it matters
types/domain.ts on ContextUnit.locator — the type the citation gate checks against, where an article locator is exactly what must never appear
.claude/project.md non-negotiable 1, the first thing an agent reads
supabase/migrations/0005_corpus.sql applied → queued in status.md as a comment on column units.locator for 0006

That is now six files for one wrong example. a-wrong-claim-has-siblings keeps earning its name; the lesson this round is that the grep has to cover code and SQL, not just prose.

3. scripts/eval-lint.ts's worked example is a fossil

Its header says:

The gold set names sources the corpus does not have yet: summa:I.q2.a3, humani-generis:36.

Three things wrong now: the Summa is ingested, the gold set was corrected to leaf locators in PR #11, and humani-generis is not in sources.yaml at all. I checked the gold set — it contains only ccc:* and leaf summa:*.

Replaced with kjv, which really is declared with no documents: block, and with the fact that matters more:

Nothing is pending today […] The branch below is still the right shape […] but nothing exercises it, so do not read a green run as proof it works.

I also caught myself mid-fix writing that "humani-generis and the other encyclicals are in sources.yaml with no documents block" — they aren't; there are exactly three sources. Verified before committing rather than after.

4. Chapter 07: the pre-flight is not the bake-off

This is the third time the pre-flight has been read as "we already tested models, so ADR-008 is decided". Chapter 07 has it right, at line 88. Its TL;DR — which is what gets read — mentioned only the bake-off. Added:

The pre-flight is not the bake-off. eval:preflight asks only can this model be served […] Passing the pre-flight is how a candidate gets admitted, never how one wins.

Three readers hitting the same wall is a documentation defect, not a reader defect.

What I checked and did not change

  • ER diagram matches 0005_corpus.sql on every field it shows; chunk_embeddings.dimensions exists
  • admin_users (0001) and rate_limit_log (0003) are where the chapter says
  • ~9k current chunks is right — 9,183 (2,865 + 2,865 + 3,453). I briefly made it exact, then reverted: a number that must change when a few hundred chunks are added is status, and ~9k is an order-of-magnitude supporting an argument
  • The Check-yourself claim that the DB holds more chunks than are current is true: 17,778 total vs 9,183 current
  • scripts/eval-lint.ts really is a correct is_current read (line 102)
  • The chunk_units join-table row from docs(guide/02): say what the join table is for, and stop claiming every article fits one chunk #23 is accurate as written
  • pgvector itself (ADR-001) is decided and not stale; what is undecided is the model, and the chapter says so correctly. Pinned the "no index yet" row to what lifts it: ADR-008's migration

Noticed, not fixed

Chapter 07's "Where this lives in code" table has a State column (✅ v0 (10), ✅, …) — the same status-outside-status.md pattern the code map just lost. Left for the chapter 07 review.

Verification

  • npx tsc --noEmit — clean
  • npx vitest run lib/citation — 24 passed (the type comment I edited is on the gate's context type)
  • npm run docs:lint — clean
  • Chapter 05: 88 prose lines. Chapter 07: 130

🤖 Generated with Claude Code

ancsingabor and others added 2 commits September 28, 2026 16:51
…e stale locator examples

**The TL;DR stated the rule without the reason.** "Superseded documents are
kept, not deleted. Every read must filter `is_current`" gives a reader a cost
and no benefit, and the benefit was two screens down in a table. A reader who
holds the rule without the reason will eventually delete the rows to save the
duplication. The reason now rides with the rule, in both places it appears.

**`summa:I.q2.a3` as a unit locator, three more sightings.** PR #24 fixed
`architecture.md` and amended ADR-002; grepping only `docs/` was the mistake.
It also lives in:

- `types/domain.ts` — on `ContextUnit.locator`, the type the citation gate
  checks against, where an article locator is precisely what must never appear
- `.claude/project.md` — non-negotiable 1, the first thing an agent reads
- `supabase/migrations/0005_corpus.sql` — applied, so queued in status.md as a
  `comment on column units.locator` for migration `0006`

**`scripts/eval-lint.ts`'s worked example is a fossil.** Its header says "the
gold set names sources the corpus does not have yet: `summa:I.q2.a3`,
`humani-generis:36`". Neither is true now: the Summa is ingested, the gold set
was corrected to leaf locators in PR #11, and `humani-generis` is not in
`sources.yaml` at all. Replaced with `kjv`, which really is declared with no
documents block — and with the fact that matters more, that the pending branch
has no members today, so a green run is not evidence that it works.

**Chapter 07 now separates the pre-flight from the bake-off in its TL;DR.**
Three times now a reader has read "we tested some models" and concluded ADR-008
was decided. The distinction was correct in chapter 07 but eighty lines down.
`eval:preflight` asks only whether a model *can be served*; it is how a
candidate is admitted, never how one wins.

Also pinned the "no vector index yet" row to what lifts it: the dimensioned
column and its index land with ADR-008's migration.

Checked and unchanged: the ER diagram matches `0005` on every field it shows,
`chunk_embeddings.dimensions` exists, `admin_users` and `rate_limit_log` are
where the chapter says, `~9k current chunks` is right (9,183), the database
really does hold more chunks than are current (17,778 v 9,183), and
`scripts/eval-lint.ts` really is a correct `is_current` read.

Chapter 05: 88 prose lines. Chapter 07: 130.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ancsingabor
ancsingabor merged commit e7b403b into main Sep 28, 2026
2 checks passed
@ancsingabor
ancsingabor deleted the docs/guide-05-review branch September 28, 2026 15:00
ancsingabor added a commit that referenced this pull request Sep 28, 2026
PR #26 narrowed ADR-008 to embeddings and recorded the generation provider as a
separate ADR still owed, but only fixed the ADR index. Step 4 of the harness
plan still read "ADR-008, the embedding and generation provider", so status.md
and `docs/adr/README.md` disagreed about the scope of the same decision.

Swept the rest: the only remaining hit for the old wording is the sentence in
`adr/README.md` that quotes the old title to explain the change, which is
correct.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant