Skip to content

feat: generate index.md and log.md deterministically, not via LLM#560

Open
AndrewDongminYoo wants to merge 6 commits into
nashsu:mainfrom
AndrewDongminYoo:feat/deterministic-index-log
Open

feat: generate index.md and log.md deterministically, not via LLM#560
AndrewDongminYoo wants to merge 6 commits into
nashsu:mainfrom
AndrewDongminYoo:feat/deterministic-index-log

Conversation

@AndrewDongminYoo

Copy link
Copy Markdown
Contributor

What

Stop letting the LLM own index.md and log.md. Both are now generated deterministically from disk after each ingest wave, inside the project lock.

Why

The stateless ingest writer let the model regenerate these two shared, cross-cutting files every wave:

  • index.md was rewritten wholesale each wave — dropping existing entries and re-referencing already-deleted pages.
  • log.md was re-emitted in full, which the append-only writer then duplicated.

Neither file needs the LLM: the index is fully derivable from page frontmatter, and the log is one app-composed line per ingest.

How

  • New index-generator.ts — pure, unit-tested functions:
    • generateIndexMd() rebuilds index.md from every page's frontmatter (grouped by type, - [[slug]] — first-sentence).
    • buildLogEntry() composes the single log line.
  • writeFileBlocks / executeIngestWrites now drop any index.md / log.md blocks the model emits, then regenerateAggregateFiles() rebuilds both from disk after all content pages land — inside the project lock, so concurrent ingests stay consistent.
  • Drop the dead stampGeneratedLogDate path and its stale schema guidance now that the log line is app-composed.
  • Surface aggregate-regen warnings in the ingest summary so a failed rebuild is visible rather than silent.

Commits

  1. feat: generate index.md and log.md deterministically, not via LLM
  2. test: cover index/log drop guard; exclude schema/purpose from index
  3. chore: drop dead stampGeneratedLogDate and fix stale schema guidance
  4. fix: surface aggregate-regen warnings in ingest summary

Testing

  • index-generator.test.ts, ingest.scenarios.test.ts, ingest-parse.test.ts — 66 pass / 0 fail.
  • tsc --build clean.

The stateless ingest writer let the LLM own index.md and log.md, two
shared cross-cutting files. Each wave the model regenerated index.md
wholesale (dropping existing entries, re-referencing deleted pages) and
re-emitted the entire log (which the append-only writer then
duplicated). Neither file needs the LLM: the index is fully derivable
from page frontmatter, and the log is one app-composed line per ingest.

- New index-generator.ts: generateIndexMd() rebuilds index.md from every
  page's frontmatter (grouped by type, `- [[slug]] — first-sentence`);
  buildLogEntry() composes the single log line. Pure functions, unit
  tested.
- writeFileBlocks + executeIngestWrites now DROP any index.md/log.md
  blocks the model emits, then regenerateAggregateFiles() rebuilds both
  from disk after all content pages land — inside the project lock, so
  "last writer regenerates" is race-free.
- Generation prompts no longer ask for index/log; index is still
  injected as read-only context for cross-linking and de-duplication.
- AGGREGATE_WIKI_PATHS narrowed to overview.md (prose, still LLM-built).

Scope held to priority 1: overview.md stays LLM-generated; existing log
duplicates are not retroactively cleaned (only new dupes are prevented).
Follow-up hardening after corpus verification (ran generateIndexMd
against the real 205-page wiki — output groups cleanly by type with
first-sentence descriptions):

- Add schema.md / purpose.md to EXCLUDED_BASENAMES so projects that
  place scaffolding pages under wiki/ don't get stray [[schema]] /
  [[purpose]] index entries.
- Add a fileExcludes assertion to the ingest scenario harness and a
  "drops-llm-index-and-log" scenario: an LLM that emits bogus index.md
  and log.md blocks must have both dropped, the index regenerated from
  page frontmatter (keeping the pre-existing page), and exactly one log
  line appended without losing prior entries. This locks the drop guard
  against a future refactor silently re-enabling LLM index writes.
- Remove the now-always-false isLog branch in the language guard (log
  paths are dropped earlier in the loop).
Cleanup after index/log generation went deterministic:

- Remove stampGeneratedLogDate() and its test. The log line is now
  composed by buildLogEntry(); nothing in the pipeline stamps an
  LLM-emitted log heading anymore.
- Rewrite the schema templates' Index/Log Format sections to state both
  files are auto-generated (don't hand-edit or ask the assistant to
  write them), and drop the "every entity should appear in wiki/index.md"
  cross-ref rule. These were injected into the generation prompt and
  contradicted the new "DO NOT output index/log" instruction.
Move the ingest warning summary/log block to run after Step 3.6
(regenerateAggregateFiles) so late-added warnings are included in
both the persisted log and the activity-panel detail.

@chuenchen309 chuenchen309 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through this — the direction is right and the implementation is clean. Deriving index.md from page frontmatter instead of asking a stateless LLM to re-emit the whole file removes the dropped-entry / deleted-page-reference / duplicated-log class of failures outright (I've been in this subsystem via the extractFrontmatterTitle cleanups). Things I verified, plus two questions.

Verified:

  • generateIndexMd is deterministic: types ordered by TYPE_DISPLAY_ORDER then unknowns A→Z (compareTypes), entries sorted by slug, aggregate/scaffolding basenames excluded. Same input → same output, which is the whole point.
  • Dropping stampGeneratedLogDate is consistent — the log line is now composed by buildLogEntry rather than post-processed out of LLM output, and the helper has no caller left on this branch (0 hits in ingest.ts).
  • Test coverage across ordering / description extraction / exclusions / type inference is thorough.

1. regenerateAggregateFiles is called from two entry points, but only one holds the project lock.

The comment at the autoIngestImpl call site says:

Runs inside the project lock (autoIngest wraps this), so "last writer regenerates from disk" is race-free.

That's true for that path — autoIngest wraps autoIngestImpl in withProjectLock (ingest.ts:581). But this PR adds a second call site in executeIngestWrites (ingest.ts:~3288), and that function doesn't take the lock — withProjectLock appears exactly twice in ingest.ts, the import and the one use at 581. executeIngestWrites is reachable from the chat panel's "Write to Wiki" (chat-panel.tsx:1361 handleWriteToWiki), while autoIngest runs from the background queue (ingest-queue.ts:745), so the two can overlap for the same project.

This matters most for log.md, which is read-modify-write:

const existing = await tryReadFile(logPath)
const entry = buildLogEntry(date, logTitle)
const next = existing.trim() ? `${existing.trim()}\n\n${entry}` : entry
await writeFile(logPath, `${next}\n`)

Two concurrent regens both read the same existing, each appends its own entry, and the second write wins — one log line silently gone. (The comment says "append one line, never rewrite prior entries", but it does rewrite the whole file.) index.md is more forgiving since it's fully derived from disk and a later regen heals it, though a regen that reads before another's pages land will write a stale index in the meantime.

Worth noting project-mutex.ts's own docstring frames the lock as covering exactly this:

Wrapping autoIngest's body in withProjectLock(projectPath, …) forces all entry points to take turns.

…and executeIngestWrites is an entry point that doesn't. Wrapping the regenerateAggregateFiles call there in withProjectLock(pp, …) would restore the property the comment claims. To be fair this race isn't created by this PR — executeIngestWrites already wrote index.md from LLM blocks without the lock — but the new comment asserts race-freedom that only holds on one of the two paths.

2. created in the generated index frontmatter.

generateIndexMd sets both created: opts.date and updated: opts.date, and both callers pass currentWikiDate(). Since the file is fully regenerated every wave, created is overwritten to the current date each run and never reflects the index's original creation — it always equals the last ingest. If it's meant to be a stable "first created" timestamp, reading the existing index.md's created (when present) and only refreshing updated would preserve it. If the index is purely a derived snapshot and both dates just mean "when this snapshot was written", current behavior is fine — just flagging which you intend.

Neither is blocking. Nice cleanup.

(Disclosure: I use an AI coding assistant. The call-site and lock claims above are ones I checked against this branch myself.)

`regenerateAggregateFiles` has two entry points but only `autoIngest`
held the project lock. `executeIngestWrites` — reachable from the chat
panel's "Write to Wiki" while a queue-driven ingest runs — called it
unlocked. `log.md` is read-modify-write, so both paths could read the
same log, each append its own entry, and the second write drop the
first.

Wrap only the regen call, not the surrounding LLM work, which would
pin the lock for the whole wave. `project-mutex` is non-reentrant, so
the lock stays at this call site rather than inside the function that
autoIngest already calls under the lock.

Also correct the log.md comment: it rewrites the file with prior
entries preserved, it does not append in place.
index.md is rewritten wholesale every ingest wave, so stamping
`created` with the current date made it always equal `updated` and
never mean "when the index was first created".

`generateIndexMd` now takes an optional `created` to carry forward;
`regenerateAggregateFiles` reads it off the existing index and falls
back to the ingest date for a brand-new one.
@AndrewDongminYoo

Copy link
Copy Markdown
Contributor Author

Both checked against the branch. Both hold — fixed in afc46bc and 5068740.

1. Lock. Confirmed. withProjectLock appears exactly twice in ingest.ts (the import and the one use at 581), and executeIngestWritesregenerateAggregateFiles at ~3288 runs outside it. The reachability is real: handleWriteToWiki (chat-panel.tsx:1354) is the only non-test caller and holds no lock, while autoIngest runs from the queue at ingest-queue.ts:745. The log.md read-modify-write is where it bites — the lost-update you described is the actual failure, and you're right that my comment claimed a property that only held on one path.

Fixed by wrapping the regen call at that site:

await withProjectLock(pp, () =>
  regenerateAggregateFiles(pp, logTitle, currentWikiDate()),
)

Two placement notes for why it's the call site and not the function: project-mutex is explicitly non-reentrant ("No re-entrancy detection"), so putting the lock inside regenerateAggregateFiles would deadlock the autoIngest path that already holds it at 1256. And wrapping the whole executeIngestWrites body would pin the lock across its LLM work for the entire wave. Also corrected the log.md comment — you're right that it rewrites the file with prior entries preserved rather than appending in place.

2. created. Correct that it always equals the last ingest today. Intent was a stable first-created date, so that's a defect, not a design choice — generateIndexMd now takes an optional created that regenerateAggregateFiles reads off the existing index, falling back to the ingest date for a new one. Two unit tests cover carry-forward and the blank/absent fallback.

One thing your review surfaced indirectly that I'm leaving out of this PR: review-view.tsx (~224-242) is a third writer doing the same unlocked read-modify-write on index.md/log.md, which contradicts this PR's "the only writer" framing. Same race class, pre-existing, separate path — tracking it separately rather than growing this diff.

Verification: tsc --build clean; index-generator / ingest-parse / project-mutex / ingest-source-path-collision all pass (83 pass). One pre-existing failure in ingest.scenarios.test.ts ("drops generated pages whose frontmatter type disagrees with schema routing") — it fails on the branch head without these commits too, so it's unrelated to the lock/created changes, but it does mean the "0 fail" claim in my PR description was wrong. Looking at it separately.

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.

2 participants