Private books in the teaching corpus; collectEntities compares by content

[lukefwalton]

What & why

Refs #24, #28. The keyless half of the demo semantic entity: the core can now ingest a whole private book as one entity with many fragments, which demo/ and example-content/ could not express. The demo wiring (the Gutenberg novel, its drafted gists, the gold route cases) is the next PR and needs a keyed demo:build.

• privateBooksDir in archive.config.ts, off by default; the example content does not change. One markdown file per book. buildPrivateBooks (src/corpus.ts) reads the frontmatter (title, about, and optionally authors, identifiers, date, version, themes, exposure, publicTitle, requireReview, fragmentBy, maxFragmentChars, firstPage) and cuts the body with the existing fragmentByHeadings / fragmentByPageMarkers into pieces whose locators are structural. A leading # Title that repeats the frontmatter title is the title page, not a chapter.
• fromPrivateBook (src/adapters/teaching.ts) yields one private entity and one fragment per piece (book:x#ch12, book:x#ch12.s3, book:x#p184). The heading goes into the fragment's text and nowhere else; publicTitle and requireReview land on the entity's policy; the section 6 metadata lint runs over the whole book before npm run index drafts or embeds anything, so a quoting title fails in the reader's output, not after a paid build. Two pieces that share a locator are refused by locator. npm run index reads the books beside the records and the notes; nothing after collectEntities changes, because the pipeline already drafts and resolves per fragment with the entity's whole text as the lint context.
• collectEntities compared two descriptions of one entity by JSON.stringify, so the same entity emitted with its properties in another insertion order was refused as "described two ways" (the nit from the 3.0.0 substrate: the archive contract, then Phase 1 in steps #23 review). It now compares by content with object keys sorted at every level; the order inside a list (creators, identifiers) is still content. Regression test added.
• A bare year in frontmatter (YAML reads date: 1900 as a number) was dropped silently by every reader; it is now the date at year precision. No shipped frontmatter uses one, so nothing in the example content or the demo changes.

Docs: README section 1 and "Make it yours" step 4, docs/CONTRACT.md §12, CHANGELOG.md. test/books.test.ts is new (invented novella fixture); the served-index test asserts that no phrase of the book survives toServedIndex and that a hit carries ch. 2, the title, the URL, and the ISBN, never the text.

Checklist

• npm test passes without an API key (the offline CI gate) — 109/109, no live calls
• npm run typecheck passes
• No change that makes the eval pass by special-casing a question — the gold set is untouched
• A boundary stays structural — a book fragment is an ordinary Fragment: its text has no field to travel in, and the new served-index test pins it
• Touched the prompt / retrieval / validation / repair? No. npm run test:dist passes and npm run demo:run holds the headline (7/7, rho 1.0000)
• Read against .github/STANDARDS.md — the new reader's messages name the file and the field, never a value

🤖 Generated with Claude Code

https://claude.ai/code/session_01F2ti456guy1bXVmjA3fBTV

---

Generated by Claude Code

[surmado-code-review]

Automated Checks (advisory, non-blocking)

✅ All checks passed.

---

Standards Compliance

This PR directly touches boundary-sensitive code (src/corpus.ts, src/adapters/teaching.ts, src/cli/build-index.ts, src/ingest/collect.ts). In the visible diff, the boundary model still looks intact: private-book prose stays in Fragment.text, fromPrivateBook() runs assertPublicSafeMetadata(entity, fragments, { path: ... }) before any draft/embed work, and the new refusal paths avoid echoing authored locator values, which matches STANDARDS §§1 and 4.

Also, the prior review nit about page-marker-derived ids being described too strongly looks fixed: the new comments in src/corpus.ts and src/adapters/teaching.ts now treat page tokens as authored strings in the linted bucket rather than “structural by construction.”

Summary

This adds a new private-book ingest path: one markdown file becomes one private entity with many fragments, and npm run index now reads those books alongside records and private notes. It also changes collectEntities() to compare entity descriptions by canonicalized content instead of raw property insertion order, plus a small YAML bare-year date fix.
Reviewer: most of the risk is in src/corpus.ts + src/adapters/teaching.ts; src/ingest/collect.ts is the other contract-defining spot.

What to pay attention to

• src/corpus.ts#buildPrivateBooks — this is where authored markdown becomes entity metadata, exposure, and structural locators. If there’s a boundary mistake in the new feature, it will likely start here.
• src/adapters/teaching.ts#fromPrivateBook — this is the boundary-adjacent conversion step that decides what remains private (Fragment.text) versus what can travel as entity/locator metadata.
• src/ingest/collect.ts#canonical — small diff, but it changes the repo-wide meaning of “same entity described two ways,” so it’s worth checking the intended array-order semantics carefully.

Things I noticed

🟡 Yellow flags — consider for this PR or a follow-up:

• src/corpus.ts

  if (typeof value === 'number' && Number.isInteger(value) && value > 0) return String(value);

  This now accepts any positive integer as a date, not just a YAML bare year. If the intended contract is specifically “year precision,” values like 1 or 20240101 could now survive as valid date strings instead of being rejected or normalized more narrowly.

Good patterns

• fromPrivateBook() doing assertPublicSafeMetadata(...) against the whole book before any draft/embed work is a strong fit for the repo’s boundary rules.
• The duplicate-locator refusal in src/adapters/teaching.ts is careful about safe error surfaces: it reports piece positions and locator schemes without echoing authored locator values.

Suggested improvements

1. Add or keep a regression test that makes the numeric-date contract explicit: either “any positive integer is accepted” or “only bare years are.”
2. If asDate() is only meant to fix YAML bare years, tighten that numeric branch so malformed numeric frontmatter still fails more narrowly.

Questions for the author

• Was the numeric-date change in src/corpus.ts meant specifically for YAML bare years, or do you want any positive integer frontmatter value to survive as date?

---

Surmado Code Review (v1.5-mt, review #4 on 58b0045) is an automated review, designed to work alongside human judgment.

Want to change your STANDARDS.md or YML? Edit it directly, or tune it with our AI agent Scout.

Comment /rerun-review on this PR to re-review the current code with your latest STANDARDS.md and config — costs 1 additional PR credit (limit 3/hour per PR).

[lukefwalton]

Verified, and it was a real exception. For a book, a chapter value is a parsed number or an ordinal and section/paragraph values are ordinals, but a page value is the author's own marker token (<<<page N>>>, one non-whitespace token), so the duplicate-locator refusal did echo an authored value. Fixed in b57167d: the message now names the two positions and the locator scheme (pieces 3 and 7 of 24 share a 'page' locator), and the test asserts a distinctive marker token never reaches the message.

On the suggested tests: withoutTitleHeading() is covered by the reader test (a # The Lantern Ledger line at the top yields three pieces, not four, and the first chapter is ch. 1); the structural-only guarantee for labels is now moot on this path because the error no longer prints a label.

---

Generated by Claude Code

[lukefwalton]

Traced it, and the honest answer was "not pinned anywhere": locatorKey() preserves a page marker's token (<<<page N>>>, any non-whitespace run), so the token sat in the fragment id, and every refusal that names a fragment prints the id, including the metadata lint's own message about that value. My first attempt to pin it (cb6a066) asserted the lint would refuse an oversized token without echoing it, which failed for exactly that reason: the message names the id. CI was red on that head for a few minutes.

963c784 closes it at the source instead. buildPrivateBooks refuses a page marker that is not a short label (letters, digits, ., -, at most 32 characters) by piece position, never echoing the token, so a page locator value, and the id built from it, is structural by construction before the lint or any id exists. The test covers both sides (x×33, a_b, p;1, 184>>> refused by position; 184, xii, A-12, 3.2 become book:w#p184 and so on), and the comment at the id now says why the key is safe to print.

---

Generated by Claude Code

[lukefwalton]

Yes, the lint sees the raw token. assertPublicSafeMetadata checks every locator value and the rendered label on every fragment ('locator value (page)', 'locator label') against the whole book, and the piece's locator is carried unchanged into the fragment; stripping the piece text does not touch it. So a page marker's token is in the linted bucket, as the standards want, and PAGE_LABEL is not what makes it safe: it only gives the lint's own refusal, which names the fragment id, a safe error surface.

The wording was the overstatement, and 58b0045 fixes it: both comments and the changelog line now say "linted, not structural", with the bound described as what it is. The page-token test already pins the id story on both sides (four markers refused by position without being echoed; four accepted and their ids spelled out); the lint's coverage of locator values is pinned in test/contract.test.ts under the metadata-lint tests.

---

Generated by Claude Code