Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/mobile/app/book/[slug].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,11 @@ export default function BookDetailScreen() {
title={book.title}
author={book.authors.map(a => a.name).join(', ') || null}
editionId={book.id}
// Both identifiers: the read tools are slug-keyed, the insight tools are editionId-keyed.
slug={book.slug}
// Already fetched for Continue Reading; the assistant was told none of it until now.
progressFraction={continuePct}
chapterTitle={book.chapters.find(c => c.slug === continueSlug)?.title ?? null}
/>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,20 @@ describe('DiscussWithAssistant', () => {
}
})

it('names an upload by bookId and a catalog book by editionId, never both', () => {
it('names an upload by bookId, and a catalog book by BOTH slug and editionId', () => {
const { unmount } = render(<DiscussWithAssistant title="D" bookId="b-1" />)
expect(decodeURIComponent(hrefOf('library.discuss.claude'))).toContain('bookId b-1')
expect(decodeURIComponent(hrefOf('library.discuss.claude'))).not.toContain('editionId')
unmount()

render(<DiscussWithAssistant title="D" editionId="e-1" />)
expect(decodeURIComponent(hrefOf('library.discuss.claude'))).toContain('editionId e-1')
expect(decodeURIComponent(hrefOf('library.discuss.claude'))).not.toContain('bookId')
// A catalog book needs both: get_book/get_chapter are slug-keyed, the insight tools are
// editionId-keyed. Sending only the id named tools that reject every call made from it, which
// is why the catalog button did not work at all until 2026-09-10.
render(<DiscussWithAssistant title="D" editionId="e-1" slug="dracula" />)
const brief = decodeURIComponent(hrefOf('library.discuss.claude'))
expect(brief).toContain('editionId e-1')
expect(brief).toContain('"dracula"')
expect(brief).not.toContain('bookId')
})

it('reads progress as a fraction, not as a percentage', () => {
Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/pages/BookDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ export function BookDetailPage() {
title={book.title}
author={book.authors.map(a => a.name).join(', ') || null}
editionId={book.id}
// A catalog book needs BOTH: get_book/get_chapter take the slug, the insight tools take
// the editionId. Sending only the id named tools that would reject every call.
slug={book.slug}
// The screen already knows where the reader stopped — the Continue Reading button above
// is built from it — and used to hand the assistant none of it.
chapterTitle={book.chapters.find(c => c.slug === continueSlug)?.title ?? null}
/>
)}

Expand Down
78 changes: 76 additions & 2 deletions packages/shared/src/lib/assistantHandoff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,90 @@ describe('buildHandoffBrief', () => {
expect(brief).not.toContain('editionId')
})

it('names a catalog book by editionId and points at the catalog tools', () => {
/**
* Which identifier each tool actually accepts, mirroring the JSON schemas in
* `McpToolCatalog` — all of which set `additionalProperties: false`, so passing the wrong one is
* a rejected call rather than an ignored field.
*
* This table is the point of the test below. The previous version asserted that the brief string
* contained the substring "get_book" — prose checked against prose — and therefore passed happily
* while the brief handed `get_book` an editionId it cannot take. The catalog Discuss button did
* not work at all, and no test noticed. Keep this in step with the schemas.
*/
const TOOL_IDENTIFIER: Record<string, ReadonlyArray<'slug' | 'editionId' | 'bookId'>> = {
get_book: ['slug'],
get_chapter: ['slug'],
get_my_book: ['bookId'],
get_my_chapter: ['bookId'],
// The insight tools take EITHER, XOR, keyed by book type: bookId for an upload, editionId for a
// catalog book. So they are satisfied by whichever one the brief is carrying.
get_my_insights: ['bookId', 'editionId'],
save_insight: ['bookId', 'editionId'],
}

/** Every tool the brief names, in order of appearance. */
const toolsNamedIn = (brief: string) =>
Object.keys(TOOL_IDENTIFIER).filter(name => brief.includes(name))

it('gives a catalog book BOTH identifiers, each next to the tools that take it', () => {
const brief = buildHandoffBrief({
title: 'Dracula',
editionId: '33333333-3333-3333-3333-333333333333',
slug: 'dracula',
})

// Read tools are slug-keyed; insight tools are editionId-keyed. A brief carrying only one of
// the two names tools that would reject every call made from it.
expect(brief).toContain('"dracula"')
expect(brief).toContain('editionId 33333333-3333-3333-3333-333333333333')
expect(brief).toContain('get_book')
expect(toolsNamedIn(brief)).toContain('get_book')
expect(toolsNamedIn(brief)).toContain('save_insight')
expect(brief).not.toContain('bookId')
})

it('never names a tool without the identifier that tool accepts', () => {
const cases = [
{
what: 'catalog',
book: { title: 'Dracula', editionId: '33333333-3333-3333-3333-333333333333', slug: 'dracula' },
present: { slug: '"dracula"', editionId: 'editionId 33333333-3333-3333-3333-333333333333' },
},
{
what: 'upload',
book: { title: 'My PDF', bookId: '22222222-2222-2222-2222-222222222222' },
present: { bookId: 'bookId 22222222-2222-2222-2222-222222222222' },
},
] as const

for (const c of cases) {
const brief = buildHandoffBrief(c.book)
const named = toolsNamedIn(brief)
expect(named.length).toBeGreaterThan(0)

for (const tool of named) {
const accepts = TOOL_IDENTIFIER[tool]
const carried = accepts.some(id => {
const marker = (c.present as Record<string, string | undefined>)[id]
return marker !== undefined && brief.includes(marker)
})
expect(
carried,
`${c.what} brief names ${tool}, which takes ${accepts.join(' or ')}, but carries none of them`,
).toBe(true)
}
}
})

it('says nothing about the connector when a catalog book has no slug', () => {
// Half the pair is worse than none: it would name slug-keyed tools with nothing to give them.
const brief = buildHandoffBrief({
title: 'Dracula',
editionId: '33333333-3333-3333-3333-333333333333',
})
expect(brief).not.toContain('get_book')
expect(brief).not.toContain('editionId')
})

it('asks the assistant to write conclusions back', () => {
// Without this line the conversation happens and nothing comes home, which is
// the entire failure mode the feature exists to fix.
Expand Down
34 changes: 23 additions & 11 deletions packages/shared/src/lib/assistantHandoff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ export interface HandoffBook {
bookId?: string
/** Edition id for a catalog book. */
editionId?: string
/**
* The catalog book's slug. Required alongside `editionId`, not instead of it: a catalog book needs
* BOTH identifiers because the tools disagree about which one they take. `get_book` and
* `get_chapter` are keyed by slug (`"required": ["slug"]`, `additionalProperties: false`), while
* `save_insight` and `get_my_insights` are keyed by `editionId`. Handing an assistant only the
* editionId — which this did until 2026-09-10 — produced a brief naming tools that would reject
* every call made from it, so the catalog Discuss button never worked at all.
*/
slug?: string
/**
* How far in, as a FRACTION of the book: 0..1, the way progress is stored
* everywhere else in this codebase ("the server stores a book-wide fraction",
Expand Down Expand Up @@ -76,19 +85,22 @@ export function buildHandoffBrief(book: HandoffBook): string {

// The identifier and the tool names. A connected client acts on this; an
// unconnected one ignores it and the conversation still works.
const id = book.bookId
? `bookId ${book.bookId}`
: book.editionId
? `editionId ${book.editionId}`
: null
if (id) {
// An upload is addressed by one id everywhere. A catalog book is not: the read tools take a slug
// and the insight tools take an editionId, so the brief has to carry both and say which is which.
if (book.bookId) {
lines.push('')
lines.push(
`If you have the TextStack connector, this book is bookId ${book.bookId}. ` +
'Read it with get_my_book and get_my_chapter, and check get_my_insights first in case we have ' +
'discussed it before. When we are done, write the conclusions back with save_insight so I find ' +
'them in the book later.')
} else if (book.editionId && book.slug) {
lines.push('')
lines.push(
`If you have the TextStack connector, this book is ${id}. ` +
(book.bookId
? 'Read it with get_my_book and get_my_chapter, and check get_my_insights first in case we have discussed it before. '
: 'Read it with get_book and get_chapter, and check get_my_insights first in case we have discussed it before. ')
+ 'When we are done, write the conclusions back with save_insight so I find them in the book later.')
`If you have the TextStack connector: read this book with get_book and get_chapter using slug ` +
`"${book.slug}", and use editionId ${book.editionId} for get_my_insights and save_insight. ` +
'Check get_my_insights first in case we have discussed it before, and write the conclusions ' +
'back with save_insight when we are done so I find them in the book later.')
}

const brief = lines.join('\n')
Expand Down
Loading