docs(design): Library app - universal ingestion into memory and collections#2056
Conversation
…collections Owner-approved direction: the Library is the single intake surface (any file, media, or URL) feeding the taosmd collections model. Cheap tier always (metadata, thumbnails, transcripts, OCR text); heavy media download opt-in with quality preference and storage accounting. Video storage strategy (low quality plus upscale, HQ keyframe references, text-region lossless screencaps) is a pre-registered research spike with explicit kill criteria, not a v1 commitment. Phased P1-P4 with backend/frontend lane split.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe new design document defines the Library as a universal input funnel, with ChangesLibrary subsystem design
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by hy3:free · Input: 32.9K · Output: 908 · Cached: 58K |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/library-app.md`:
- Around line 32-35: Update the ingest endpoint in the design flow to match the
existing POST /api/knowledge/ingest contract, or explicitly document P1’s chosen
migration or compatibility-alias strategy if /api/library/ingest is intentional.
Ensure the documented endpoint and the implementation/tests target the same
contract.
- Around line 41-50: Expand the collections handoff design to define stable
artifact and collection identifiers, plus an outbox or resumable state machine
covering file writes, indexing, linking, and granting. Assign each operation a
durable idempotency key so retries after partial failure resume safely without
duplicating artifacts or records or reapplying grants, and replace the narrower
“per (item, stage)” guarantee with this end-to-end behavior.
- Around line 8-12: Update the Library URL-ingestion design to require an
explicit safety boundary before invoking yt-dlp: allow only approved schemes,
validate DNS and every redirect target against blocked/private IP ranges,
restrict outbound network access, enforce connection and download timeouts, and
apply sandboxed CPU, memory, process, and disk limits. State that these controls
apply to all URL-ingestion paths.
- Around line 108-114: Update the roadmap sections describing text-region
screencaps and the related P4 entries so OCR is assigned to the appropriate
product phase, such as P1 or P2, rather than shipping with the P4 spike. Keep P4
scoped exclusively to video-storage hypotheses and evaluation, while preserving
OCR ingestion and collection indexing as an independently shippable capability.
- Around line 46-50: Resolve the scope mismatch in the library design by
aligning the universal cheap-processing claim and processor registration list
with P1’s committed file, text, PDF, and image support. Either narrow automatic
ingestion to those P1 kinds or explicitly mark audio, video, and archive
processors as later-phase work, and update the related acceptance/delivery
statements consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e87fbe2-fb3b-474b-b209-6678abab6bf6
📒 Files selected for processing (1)
docs/design/library-app.md
| The Library is the dumping ground. Any info, file, or media in any format gets | ||
| dropped in (drag-drop, share sheet, URL paste, agent send), and the Library | ||
| processes and/or ingests it into memory and collections. Agents and apps are | ||
| then granted different access levels to the relevant collections. Nothing else | ||
| in taOS accepts arbitrary input; everything funnels here. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Specify the network and resource-safety boundary for URL ingestion.
The Library accepts arbitrary URLs and invokes yt-dlp; the design must require scheme validation, redirect-aware DNS/IP checks, restricted egress, download timeouts, and sandbox/resource limits. Without these controls, a URL can target internal services or consume unbounded network, CPU, and disk resources.
Also applies to: 64-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/library-app.md` around lines 8 - 12, Update the Library
URL-ingestion design to require an explicit safety boundary before invoking
yt-dlp: allow only approved schemes, validate DNS and every redirect target
against blocked/private IP ranges, restrict outbound network access, enforce
connection and download timeouts, and apply sandboxed CPU, memory, process, and
disk limits. State that these controls apply to all URL-ingestion paths.
| POST /api/library/ingest -> artifacts(item_id, kind, path, meta_json) | ||
| | jobs(item_id, stage, state, error) | ||
| v | ||
| Ingest pipeline (async, per-kind processors, job rows drive retries) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Align the documented ingest endpoint with the existing API contract.
The design names POST /api/library/ingest, but the supplied implementation and tests use POST /api/knowledge/ingest (tinyagentos/routes/knowledge.py:83-109, tests/test_knowledge_routes.py:53-66). Specify whether P1 introduces a new endpoint, migrates the existing one, or provides a compatibility alias; otherwise clients and tests will target different contracts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/library-app.md` around lines 32 - 35, Update the ingest endpoint
in the design flow to match the existing POST /api/knowledge/ingest contract, or
explicitly document P1’s chosen migration or compatibility-alias strategy if
/api/library/ingest is intentional. Ensure the documented endpoint and the
implementation/tests target the same contract.
| Collections handoff: write text artifacts into a per-target folder under an | ||
| allowed root, then taosmd collections index; link to project; grants stay | ||
| EXPLICIT (no transitivity - same rule as the collections verdict). | ||
| ``` | ||
|
|
||
| - One `LibraryStore` following the SCHEMA/MIGRATIONS discipline. | ||
| - Processors are registered per detected kind (url:youtube, url:web, pdf, | ||
| image, audio, video, text, archive). Detection at ingest, stored on the item. | ||
| - Every artifact records provenance: source URL, fetch time, processor | ||
| version. Reprocessing is idempotent per (item, stage). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Define idempotency across the collections handoff.
“Idempotent per (item, stage)” is insufficient for the multi-step sequence of writing files, indexing, linking, and granting. A retry after a partial failure can create duplicate artifacts or collection records, or reapply grants. Specify stable artifact/collection identifiers and an outbox or resumable state machine with per-operation idempotency keys.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~47-~47: The official name of this popular video platform is spelled with a capital “T”.
Context: ...s are registered per detected kind (url:youtube, url:web, pdf, image, audio, video, t...
(YOUTUBE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/library-app.md` around lines 41 - 50, Expand the collections
handoff design to define stable artifact and collection identifiers, plus an
outbox or resumable state machine covering file writes, indexing, linking, and
granting. Assign each operation a durable idempotency key so retries after
partial failure resume safely without duplicating artifacts or records or
reapplying grants, and replace the narrower “per (item, stage)” guarantee with
this end-to-end behavior.
| - One `LibraryStore` following the SCHEMA/MIGRATIONS discipline. | ||
| - Processors are registered per detected kind (url:youtube, url:web, pdf, | ||
| image, audio, video, text, archive). Detection at ingest, stored on the item. | ||
| - Every artifact records provenance: source URL, fetch time, processor | ||
| version. Reprocessing is idempotent per (item, stage). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Resolve the cheap-tier and P1 scope contradiction.
The document says cheap processing is automatic for “everything” and registers processors for audio, video, and archives, but P1 only commits to file/text/pdf/image processors. Narrow the universal-ingestion claim or explicitly phase the remaining cheap processors so the acceptance and delivery boundaries are implementable.
Also applies to: 125-128
🧰 Tools
🪛 LanguageTool
[uncategorized] ~47-~47: The official name of this popular video platform is spelled with a capital “T”.
Context: ...s are registered per detected kind (url:youtube, url:web, pdf, image, audio, video, t...
(YOUTUBE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/library-app.md` around lines 46 - 50, Resolve the scope mismatch
in the library design by aligning the universal cheap-processing claim and
processor registration list with P1’s committed file, text, PDF, and image
support. Either narrow automatic ingestion to those P1 kinds or explicitly mark
audio, video, and archive processors as later-phase work, and update the related
acceptance/delivery statements consistently.
| 3. **Text-region lossless screencaps.** At ingest, sample frames, run text | ||
| detection/OCR, score text density, and save the top text-heavy segments as | ||
| lossless stills PLUS their OCR text. Two payoffs: the stills guarantee | ||
| nothing legible is lost to compression, and the OCR text goes straight into | ||
| the collection where it is worth more to agents than the pixels. The OCR | ||
| path ships with the spike regardless of how 1 and 2 measure, because it is | ||
| cheap and its value does not depend on the upscaling outcome. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make OCR independent of the P4 research spike.
The objective says OCR is shippable independently, but this section says the OCR path ships “with the spike,” while P4 is defined as the video-storage spike. Move OCR into the relevant product phase (for example P1/P2) and keep P4 limited to the storage hypotheses and evaluation.
Also applies to: 133-134
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/library-app.md` around lines 108 - 114, Update the roadmap
sections describing text-region screencaps and the related P4 entries so OCR is
assigned to the appropriate product phase, such as P1 or P2, rather than
shipping with the P4 spike. Keep P4 scoped exclusively to video-storage
hypotheses and evaluation, while preserving OCR ingestion and collection
indexing as an independently shippable capability.
The Library app spec Jay queued: one intake surface for any info, file, or media, processing everything into the approved taosmd collections model with explicit grants.
Key decisions locked:
Slice issues and board cards follow this PR.
Summary by CodeRabbit