feat: extract_text, and the five UI gaps behind a working API - #21
Merged
Merged
Conversation
`extract_text` is listed in plan-of-attack's M6 job table as though it exists. It never did: no KIND_ constant, no module. So `enrichment/source.py` raised NoSourceMaterial for every document, saying the gap out loud, and summarize/autotag/describe refused a PDF outright. PDF text comes from pypdfium2, already pinned for first-page thumbnails — reusing it keeps the licensing argument in thumbnails.py settled rather than reopening it for pypdf. Office formats need python-docx, python-pptx and openpyxl; they are pure-Python wheels and their one compiled transitive dependency, lxml, ships cp313 wheels, so nothing builds from source on the pinned interpreter. Text lands in its own table rather than a column on Asset. SQLModel selects every column of every row and the library listing selects assets by the page, so a 200-page PDF's text in an asset column would be read from disk to render a thumbnail grid that never looks at it. One row per page, slide, sheet or chunk, which is also the unit that will be embedded when full-document search lands. The precedence detail worth keeping: the document branch sits *above* the poster branch in `gather`, not down with the old refusal. Every PDF gets a first-page thumbnail at ingest, so one placed lower would leave a PDF summarised from a picture of its cover — the same mistake the transcript-beats-poster rule at the top of that module exists to prevent, one format over. Bulk gets extract_text where it does not get transcription: it calls nothing and bills nothing, so the mis-click that makes transcription too expensive to offer costs only time. `embed` is deliberately not chained — page bodies are not vectorised yet, so it would queue a job that does no new work. Full-body document search is not in this change. The AI summary reaches asset_fts and the embeddings the usual way, so documents become findable by summary; the raw body does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6D8S8oXYij748Fu5k4AAZ
Each of these is a backend endpoint or a store action with no component behind it — code
that passes every test and that no user can get to.
**Tag management.** M3 built rename, delete and a nested category tree, then wired a
picker that can only add. A user could type a tag into existence and never rename, file
or delete it, so the recursive-CTE tree was read-only from the UI. The panel lives in
Settings: this is housekeeping, done rarely, and the header already carries five
controls. `tagsApi.recategorise` and `updateCategory` had no store action at all; they do
now, and the two rejections the server bothers to explain — a duplicate name, a category
moved inside itself — are shown rather than swallowed.
While there: the rollback paths in `stores/tags.ts` wrote a pre-await snapshot back with
no staleness check, so a failure landing after sign-out repopulated an emptied store with
the previous user's vocabulary. A tag screen exercises those paths directly.
**`/a/{id}`.** GN-4 settles the Notes→GAM asset reference as a plain link to this path,
and chose that over a custom BlockNote block partly because it needed no work in Notes —
reasoning that assumed this end existed. It did not: `/a/anything` fell through the
catch-all to the library with no sign anything had gone wrong. `openById` inserts the
fetched asset into the library store, because `update`, `remove`, `setAssetTags` and
`refreshAsset` all map over that list and would otherwise be controls whose every write
landed nowhere.
**Search filters.** `searchApi.run` typed `asset_type` and `limit`; the view passed
neither. The type chips are lifted out of FilterBar, which is not reusable whole — it is
wired to the library store for all nine of its filters. "Show more" asks for a bigger
page rather than an offset, because `routers/search.py` has no offset and its `total` is
the size of the page, not the corpus. That is a backend change, and it is not here.
**401 mid-session.** There was a request interceptor and no response interceptor, so an
expiry during an upload surfaced as an inline error and nothing re-authenticated. The
handler is injected from App rather than imported, since the store imports `api/auth.ts`
which imports the client. It fires once — the activity store polls every two to ten
seconds — and only for 401, never for the 403 the CSRF guard raises. It returns whether
it acted, so the ordinary anonymous 401 during bootstrap does not spend the one shot.
**Sign out.** `signOut()` had existed since M2 and was called by nothing outside its own
test, so leaving a session meant clearing localStorage by hand.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6D8S8oXYij748Fu5k4AAZ
The outstanding list is only worth anything if it is true. `extract_text` and the four UI gaps are struck through rather than deleted, because a later session reading "no tag-management screen" and finding one needs to know it was built, not wonder whether it was ever missing. Three things are recorded as still open, each of which is easy to mistake for done: - Document text is stored and searchable nowhere. A document *is* findable by the summary enrichment writes, because that path reindexes — which looks exactly like the body being indexed until someone searches for a phrase on page 74. - Search has no offset, and its `total` is the size of the page rather than the corpus, so "show more" is a bigger page and not a pager. - GN-4 now works from this end. Notes still has to write the links. The M6 job table described `extract_text` as pypdf; it is pypdfium2, and the reason is worth keeping — that library is already here for thumbnails, and its docstring records why it was chosen over the AGPL and poppler-dependent alternatives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6D8S8oXYij748Fu5k4AAZ
davior
marked this pull request as ready for review
September 16, 2026 13:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes six gaps found by auditing the code against
docs/plan-of-attack.mdrather than trusting it. Every one of them sat inside a milestone already marked done, which is why none of them was visible to anyone reading the plan.extract_textplan-of-attack.mdlisted it in M6's job table as though it existed. It never did — noKIND_constant, no module — soenrichment/source.pyraisedNoSourceMaterialfor every document andsummarize/autotag/describerefused a PDF outright..pdf.docx.pptx.xlsxread_only=True.txt.md.csvDependencies. PDF needs nothing new: pypdfium2 is already pinned for first-page thumbnails, and reusing it keeps the licensing argument in
ingest/thumbnails.pysettled rather than reopening it for pypdf. The three Office libraries are pure-Python wheels whose only compiled transitive dependency is lxml, which ships cp313 wheels — checked on PyPI before adding them, since a source build on the pinned 3.13 is how Pillow and numpy break.Storage. A new
DocumentPagetable, not a column onAsset. SQLModel selects every column of every row and the library listing selects assets by the page, so a 200-page PDF's text in an asset column would be read from disk to render a thumbnail grid that never looks at it. One row per natural unit is also the granularity page-level search will want.The precedence detail worth reviewing. The
FROM_DOCUMENTbranch ingathersits above the poster branch, not down where the old refusal was. Every PDF gets a first-page thumbnail at ingest, so a branch placed lower would leave a PDF summarised from a picture of its cover — the same mistake the transcript-beats-poster rule at the top of that module exists to prevent, one format over. There is a test for exactly this.embedis deliberately not chained after extraction: page bodies are not vectorised under this change, so it would queue a job that does no new work. The one-line change is in place and commented for when that lands.The five UI gaps
Each was a backend endpoint or store action with no component behind it — code passing every test that no user could reach.
TagPanelin Settings.tagsApi.recategoriseandupdateCategoryhad no store action at all. The two rejections the server bothers to explain — duplicate name, category moved inside itself — are shown rather than swallowed. Also guarded the store's rollback paths, which wrote a pre-await snapshot back with no staleness check, so a failure landing after sign-out repopulated an emptied store with the previous user's vocabulary./a/{id}. GN-4 settles the Notes→GAM asset reference as this path, chosen over a BlockNote shortcode partly because it needed no work in Notes — reasoning that assumed this end existed./a/anythingfell through the catch-all to the library with no sign anything was wrong.openByIdinserts the fetched asset into the library store, becauseupdate,remove,setAssetTagsandrefreshAssetall map over that list and would otherwise be controls whose every write landed nowhere.asset_typeandlimit. Typed bysearchApi.run, passed by nothing. Both now survive a reload in the URL.Apprather than imported (the store importsapi/auth.ts, which imports the client). Fires once — the activity store polls every 2–10s — only on 401, never on the CSRF guard's 403, and it returns whether it acted so the ordinary anonymous 401 during bootstrap does not spend the one shot.signOut()had existed since M2 with no caller outside its own test.Deliberately not in this change
Recorded in
plan-of-attack.mdrather than left to be rediscovered:segment_ftsnor the vector index. A document is findable by the summary enrichment writes, because that path reindexes — which looks exactly like the body being indexed until someone searches for a phrase on page 74.routers/search.pyhas nooffsetand itstotalis the size of the page, not the corpus. "Show more" asks for a bigger page, capped at the server's 100. A real pager is a backend change..doc,.xls,.ppt,.rtf,.odt,.ods,.odp) refuse by name with what to do instead. Scans need OCR, which is its own project.Testing
npm run build,eslint --max-warnings 0andprettier --checkall clean.Not verified here: nothing touching real files in a browser, since this sandbox has no ffmpeg. The end-to-end checks are listed in the plan — the one that matters most is running Summarize on a PDF and confirming the summary reflects its contents rather than its cover page.
🤖 Generated with Claude Code
https://claude.ai/code/session_01F6D8S8oXYij748Fu5k4AAZ
Generated by Claude Code