feat(api): library import onboarding — POST /libraries/:id/import (O-8) - #247
Conversation
O-8 onboarding targets an explicit ADR-015 library; the source-derived default resolution remains the behavior when libraryId is omitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Separate from parse jobs so /parse/jobs keeps its ParseJobResult schema; the onboarding report carries style derivation, an editability summary, and parse warnings (O-8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pure pre-order walk over a classified tree → per-value counts + a low-confidence review list (O-8 report §editability). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
POST handler + async job orchestration: parse -> persist into the target library -> derive DOCX style template -> classify editability -> assemble the three-section onboarding report. Non-DOCX flags styleSourceNeeded rather than failing. Bytes discarded after parse (ADR-021). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Routes registered with the shared parse rate-limit + multer; openapi.yaml gains the path and OnboardingJob/Report/EditabilitySummary schemas so the contract gate stays green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers every acceptance criterion: DOCX three-section report + DB state, .sec styleSourceNeeded, 404 unknown library, 400 malformed upload + bad library id. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Satisfy sonarjs/no-alphabetical-sort (explicit localeCompare comparator) and check in the O-8 implementation plan. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 38 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughImplements the O-8 library import onboarding feature: a new async ChangesLibrary Import Onboarding (O-8)
Sequence Diagram(s)sequenceDiagram
participant Client
participant importLibraryHandler
participant processOnboardingJob
participant parsePool
participant persistParsedSpec
participant deriveStyleIfDocx
participant classifyAndSummarize
participant onboardingJobs
Client->>importLibraryHandler: POST /libraries/:id/import (multipart file)
importLibraryHandler->>importLibraryHandler: validateUpload (ext/MIME/zip checks)
importLibraryHandler->>onboardingJobs: createOnboardingJob()
importLibraryHandler-->>Client: 202 { jobId }
importLibraryHandler-)processOnboardingJob: schedule async (fire-and-forget)
processOnboardingJob->>parsePool: parse upload off-thread
parsePool-->>processOnboardingJob: SpecTree + refs
processOnboardingJob->>persistParsedSpec: persist with explicit libraryId + SHA-256 originMeta
persistParsedSpec-->>processOnboardingJob: specId
alt DOCX input
processOnboardingJob->>deriveStyleIfDocx: analyze styles, upsertOnboardedTemplate, link template
deriveStyleIfDocx-->>processOnboardingJob: { templateId, derivationReport }
else non-DOCX input
processOnboardingJob->>deriveStyleIfDocx: skip derivation
deriveStyleIfDocx-->>processOnboardingJob: { templateId: null, derivationReport: null }
end
processOnboardingJob->>classifyAndSummarize: classify editability, summarizeEditability
classifyAndSummarize-->>processOnboardingJob: EditabilitySummary
processOnboardingJob->>onboardingJobs: updateOnboardingJob(complete, OnboardingJobResult)
Client->>importLibraryHandler: GET /libraries/import/jobs/:jobId
importLibraryHandler->>onboardingJobs: getOnboardingJob(jobId)
onboardingJobs-->>importLibraryHandler: OnboardingJob
importLibraryHandler-->>Client: 200 OnboardingJob
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Re-importing the same master returns the same specId (persistParsedSpec ON CONFLICT upsert), so the deterministic template name collided and the 23505 catch swallowed the freshly derived rules — the report then falsely reported styleSourceNeeded:true / templateId:null while the stale template kept its old rules. Now upsertOnboardedTemplate refreshes the existing template's rules (bulkUpsertTemplateRules) and re-links it, so a re-imported (possibly edited) master ends with one current template and an accurate report. The 23505 path remains as a concurrent-create fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/api/onboarding.ts`:
- Around line 234-237: In the catch block where updateOnboardingJob is called
after an error occurs, the update only sets the status to 'failed' and includes
the error message, but does not explicitly set the progress stage and percentage
fields. To fix the inconsistent job payload, modify the updateOnboardingJob call
to also include progress stage and percentage values that indicate a failed
state, ensuring the job payload is consistent for polling clients when an error
is caught.
- Around line 133-135: The workerRaw data from the worker thread pool is cast
directly to WorkerOutput type without runtime validation, which violates the
requirement to validate external input crossing process boundaries. Replace the
type assertion with a Zod schema validation. Create a Zod schema that matches
the WorkerOutput interface structure and use it to parse and validate workerRaw
before destructuring the tree and refs properties. This ensures malformed data
from the worker thread cannot propagate into downstream persistence and
classification operations.
In `@src/lib/jobs.ts`:
- Around line 2-4: The import statement for ParseWarning bypasses the AST module
barrel by importing directly from types.js instead of through the index.js
barrel export. Update the import statement to import ParseWarning from
../ast/index.js instead of ../ast/types.js. Then verify that ParseWarning is
re-exported from src/ast/index.ts by adding the re-export from ./types.js if it
doesn't already exist. Apply this same pattern correction to the other files
mentioned (src/api/onboarding-report.ts and src/api/onboarding.ts) that have
similar direct imports of ParseWarning from internal modules.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 66f193da-3b38-4e2f-97d1-7b4a66246e09
📒 Files selected for processing (12)
docs/superpowers/plans/2026-06-23-library-import-onboarding.mdopenapi.yamlsrc/api/contract.integration.test.tssrc/api/onboarding-report.test.tssrc/api/onboarding-report.tssrc/api/onboarding.integration.test.tssrc/api/onboarding.tssrc/api/router.tssrc/db/queries/specs.integration.test.tssrc/db/queries/specs.tssrc/lib/jobs.test.tssrc/lib/jobs.ts
Lets the lib/api layers consume them through the module barrel instead of reaching into ast/types.js / ast/schemas.js internals (module-boundary rule). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…l failed stage CodeRabbit review of #247: - Validate the cross-thread worker output with Zod before use (was a bare `as WorkerOutput` cast of unknown). Extracts the single-source-of-truth workerOutputSchema into lib/parse-worker.ts, consumed by both parse.ts and onboarding.ts (DRY) — a malformed payload now fails the job cleanly with a cause-chained ZodError instead of slipping through uncast. - Set stage:'failed' (pct:100) on the onboarding failure update so polling clients never see status:'failed' stranded on the last running stage. - Import ParseWarning / SpecTree / SpecNode through the ast barrel, not ast/types.js internals (module-boundary rule). Tests: parse-worker.test.ts pins the schema (accepts good, rejects malformed); onboarding.test.ts pins the boundary-failure contract (malformed worker output → status+stage 'failed' with a non-empty error). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#325) * docs(readme): sync capabilities to last month of merged PRs Reflect shipped work in the README's "Included Today", "API Surface", and MCP tool table, validated against the merged diffs and current main: - PDF ingest (text-layer + OCR + font-encoding recovery) accepted by POST /parse (#287, #290, #311) - coordination / E&O report + submittal register (#241, #269, #277, #282, #283, #284) and article-role tagging (#273) - onboarding pipeline: library import, editability review/override, reclassify, finalize/reopen, open-comments (#243, #247, #248, #249, #272) - spec/project soft-delete + restore (#257, #313), document concurrency (#197), revision/addendum manual rendering (#221), numbering profiles (#317, #322) - add missing MCP tools get_numbering_profile, submittal_register, open_comments_report; document GET /docs (Scalar) (#213, #285) - add Example Client pointer to examples/web_ui_demo (#225) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(roadmap): move shipped work to done; re-date to 2026-07-01 Reconcile the roadmap with merged reality (was stamped 2026-06-17). Moved from planned/in-progress to Included, each validated against the diff: - PDF ingest (#287, #290, #311) — remove from "Later" - deep paragraph nesting pr6/pr7 (#215) - revision nomenclature (#216) + revision/addendum manual rendering (#221) — the two "Near Term" Phase 2e items are done - coordination / E&O report, required-sections, article-role, submittal register (#239, #241, #269, #273, #277, #282, #283, #284) — new "Coordination and Semantics" section; removed "coordination report" from planned Phase 4 - onboarding APIs (#243, #247, #248, #249, #272) — API done; UI remains planned - soft-delete/withdraw (#257, #313), section-number format (#266, #271), external-content associations (#242), structural numbering profiles (#317) Kept as planned (foundation only): header/footer composition (#222, #314) and keynote surfacing (#315) — DB/AST exist, no resolution/render/export yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): reflect merged structural changes Update the architecture spec for shipped work, validated against the diffs and current schema/routes: - Tech Stack + Data Flow: Parse — PDF text-layer (unpdf/pdfjs-dist) + OCR (tesseract.js/@napi-rs/canvas) path and numberingProfileId override (#287, #290, #311, #317; ADR-034, ADR-039) - DB schema — specs.onboarding_status/withdrawn_at, projects.section_number_format /deleted_at/deleted_by, paragraphs.source_facts/classification/ editability_override; "Additional tables" summary for editing_conventions, paragraph_associations, required_sections, keynotes, header_footer_configs, numbering_profiles, revision_nomenclature_profiles (foundation-only tables flagged) (ADR-021/022/023/028/031/032; #187, #242) - new Coordination Report / E&O section (finding vocabulary) and Document Concurrency section (locks/optimistic/lifecycle) (#197, #241, #269, #277, #282, #283, #284; ADR-018, ADR-033/035/036/037) - AST meta.articleRole (#273, ADR-033); API-surface note pointing at the CI-enforced openapi.yaml + GET /docs; refreshed MCP tool list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
The onboarding entry point for the editability program (O-8 of the onboarding &
editability design): one endpoint that takes a master document and produces a
reviewable library master. Headless-first (ADR-002) so curl/agents/integration
tests can drive the whole flow before any UI exists. Extends the surface of
WT-3's
POST /templates/import(consensus style derivation) with persistenceinto an ADR-015 library + editability classification.
What
New
POST /libraries/:id/import— multipart upload (DOCX/SEC/TXT), async via theexisting
lib/jobs.tspattern. It orchestrates already-tested building blocks:parsePool) withper-paragraph source-fact capture.
persistParsedSpecnowaccepts an explicit
libraryIdthat overrides source-derived resolution).style_templatesrowlinked to the spec. Non-DOCX → the report flags
styleSourceNeeded: true(assign later via O-12 / ADR-015 inherit), it does not fail.
built-in
Industry Default(reusesreclassifySpec).editability summary (per-value counts + low-confidence review list) + parse
warnings.
Source bytes are discarded after parse (ADR-021); only
origin_meta(sha256 + sanitized filename) is recorded.
openapi.yamlgains the path and theOnboardingJob/OnboardingReport/EditabilitySummaryschemas so the CIcontract gate stays green.
Poll progress at
GET /libraries/import/jobs/:jobId(a 3-segment literal routethat can never shadow the 2-segment
/libraries/:id/...routes).Out of scope (per the issue):
onboarding_statusfinalize/reopen (O-11),corrections/reclassify API (O-9, already merged separately), MCP parity (O-13).
Design decisions
Ambiguous calls I made and documented, so the reviewer can flag any wrong turn:
src/api/onboarding.tsinstead of growinglibraries.ts. The pipelineis substantial and distinct;
libraries.tsstays focused and both stay underthe 400-line cap. Mirrors
parse.tsbeing separate fromspecs.ts.lib/jobs.ts(ownMap+ result type)rather than reusing
ParseJobResult. The three-section onboarding report doesnot fit the parse contract, and
/parse/jobsmust keep its existing schema.reclassifySpec(specId, {})for the classify step — it already resolvesthe library profile (or built-in default) and stores classifications, so the
endpoint stays pure orchestration with no duplicated classify logic.
analyzeDocxStyles→deriveTemplate→createTemplateWithRules→setSpecStyleSource). Aduplicate template name on re-import (pg 23505) is treated as non-fatal — the
derivation report still surfaces; any other DB error fails the job loudly.
reasonable default; trivially adjustable (
summarizeEditabilitytakes it as aparameter).
RESPONSE_ALLOWLIST, mirroring howpost /parse+get /parse/jobs/{}are handled — behavior is asserted in thededicated
onboarding.integration.test.ts, not the structural contract file.Testing
pnpm test→ 1005 passing withDATABASE_URLset, as CI provides)pnpm test:integration— onboarding/contract/parse/libraries/specs all green).secfixture → 53 paragraphs,styleSourceNeeded: true, no template; unknown library → 404; bad extension / bad library id → 400🤖 Co-authored by Claude Opus 4.8. Closes #135.
Summary by CodeRabbit
Release Notes