Skip to content

Agent-created sections bypass the sectionId factory and use a non-canonical, positional id #834

Description

@gbergengruen

Follow-up from the #660 reordering stack (#784 in particular). Not introduced by that stack — this predates it — but #784 establishes sectionId immutability, and this is a live allocation site that sits outside the new factory.

What

packages/agents/src/tools/book-tools.ts allocates section ids for agent-created sections as:

const nextIndex = existingSectioning.sections.length
const sectionId = `${pageId}_s${nextIndex}`

There is a second identical site further down the same file (~line 595).

This runs in production: packages/agents is wired into the API through apps/api/src/services/agents-service.ts.

Two problems

1. The id is positional. nextIndex is sections.length, so it is derived from the array's current size rather than from a high-water mark. Delete a section and add a new one and the id is reissued — the exact failure mode #784 fixed for clone / split / merge / delete, where a reused id silently inherits the retired section's sign_language_videos.section_id, toc-generation entry and ${sectionId}_ans_* text-catalog keys (and so its translations and generated TTS audio).

2. The format is not canonical. These are pg001_s0, not pg001_sec001. #784's parser

export function parseSectionId(id: string): { pageId: string; seq: number } | null {
  const match = /^(.+)_sec(\d+)$/.exec(id)
  ...
}

returns null for _s0. So an agent-created section is invisible to createSectionIdFactory's high-water-mark scan. The two id namespaces do not collide with each other today, but the agent-created ids get none of the no-reuse protection, and any code that treats parseSectionId as a type guard will silently skip these sections.

Suggested fix

Route both sites in book-tools.ts through createSectionIdFactory / formatSectionId so agent-created sections get canonical, never-reused ids like every other creation path.

Worth deciding separately what to do about books that already contain _sN ids in the wild — as with #784, they must not be auto-remapped, since a wrong guess reattaches a sign-language video to unrelated content.

Notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions