feat(literature): add reference library and research workflows - #2236
feat(literature): add reference library and research workflows#2236ewen-poch wants to merge 3 commits into
Conversation
Codex ReviewVerdict: needs changes [P1] Keep the lockfile aligned with direct runtime dependenciespackage-lock.json:27 Impact: The root lockfile omits the direct Recommendation: Regenerate or correct Summary: Static review found a lockfile inconsistency that can break clean reproducible installs. |
|
Fixed in 66c9fad. Restored main's direct Validation: |
Codex ReviewVerdict: needs changes [P1] Preserve literature artifact mentions during composer DOM parsingsrc/renderer/src/pages/workspace/composer/composer-doc.ts:394 Impact: Literature artifact chips are emitted but rejected during DOM-to-document synchronization, so editing or rehydrating a message can silently drop the literature PDF reference. Recommendation: Allow [P2] Allow corrupted content blobs to be repairedsrc/main/storage/content-repository.ts:173 Impact: If the destination already exists with corrupted bytes, the new copy is ignored, the blob is quarantined, and repeated publication of the same source fails indefinitely. Recommendation: Verify an existing destination on [P2] Enforce unique root collection namessrc/main/database/migrations/0029-literature-foundation.ts:118 Impact: SQLite allows multiple NULL values in the composite unique index, so duplicate root collections with the same normalized name can be created despite the declared sibling-uniqueness constraint. Recommendation: Use a root-specific partial unique index plus a child index, or a non-null root sentinel, and enforce the invariant in the write path. Summary: Three concrete regressions were found in the literature/storage integration. |
|
Fixed the PDF mention and corrupt-content findings in 3546231:
Validation: all 61 tests across the two owning suites passed; full typecheck, targeted ESLint, and The root Collection uniqueness recommendation is pending owner confirmation. Enforcing it would change a database constraint and requires an explicit policy for existing same-name root Collections, plus migration work. No collection behavior, schema, or migration has been changed for that finding. |
Codex ReviewVerdict: needs changes [P1] Library navigation bypasses dirty-preview protectionsrc/renderer/src/stores/navigation-store.ts:235 Impact: openLibrary, and the adjacent openLiteratureItem action, navigate directly without invoking requestPreviewLeaveForNavigation. Activating literature actions from a workspace can therefore unmount a dirty PDF/file preview and silently discard unsaved edits. Recommendation: Route these navigation actions through requestPreviewLeaveForNavigation, matching goHome, openProjectLiterature, and openCollectionLiterature. [P2] Literature metadata requests bypass the configured network stacksrc/main/ipc.ts:1546 Impact: Production construction of LiteratureMetadataEnricher leaves its fetch function at global fetch. In environments requiring the system/VPN proxy, Crossref and PubMed enrichment uses Node's direct network path and can fail even though the application provides netFetchStandard for proxy-aware main-process requests. Recommendation: Inject netFetchStandard when constructing LiteratureMetadataEnricher, while retaining dependency injection for tests. [P2] Reference resolution also bypasses proxy-aware fetchingsrc/main/acp/runtime-composition.ts:258 Impact: The production LiteratureReferenceResolver is constructed with its default global fetch, so DOI/PMID resolution for the save-references workflow similarly ignores configured system/VPN proxy settings and can fail in supported proxied environments. Recommendation: Construct LiteratureReferenceResolver with netFetchStandard (or change its production default to that helper) and keep an injectable fetcher for tests. Summary: Branch and pull-request title prechecks passed. Static inspection found two merge-blocking integration defects; no tests, lint, typecheck, build, or project commands were run. |
Add the Literature schema, content storage, metadata and full-text discovery, duplicate handling, citation finalization, and scoped MCP integration. Preserve attachment and reading-source identity across artifact versions.
Add reference import, metadata and PDF review, duplicate comparison and batch operations, citation styles, and Collection mentions. Integrate Reading and Artifact Literature provenance, improve navigation and nested dialogs, and translate the renderer in eight locales.
3546231 to
db507f5
Compare
|
Addressed the actionable findings and CI failures:
Targeted catalog, metadata, resolver, and translation tests passed (851 tests); library/navigation tests passed (143 tests); ACP/network tests passed (6 tests). Migration/ledger tests, typechecks, source lint, generated schema, and web API map checks passed. The branch is consolidated into three commits. Internal planning documents have been removed from the entire PR commit history and remain local only. |
Codex ReviewVerdict: needs changes [P2] ContentBlob records survive project deletionprisma/schema.prisma:416 Impact: Deleting a project removes its artifact/upload rows and files, but ContentBlob has no project ownership or cascade cleanup. This leaves available blob records pointing to missing storage, leaking database authority rows until a later startup sweep. Recommendation: Update project deletion to collect and remove unreferenced ContentBlob records after deleting domain references, using the existing reference-aware sweep so shared blobs are preserved. Summary: Branch and title prechecks passed. Found one project-deletion lifecycle defect. |
|
The current head passed all three Ubuntu full-test shards, Static checks, Windows core, and Windows E2E shard 2. The macOS build, functional/workspace E2E, accessibility, and native tests also passed. The remaining CI issues are:
The new ContentBlob project-deletion finding is valid: version authority is removed without immediate cleanup of corresponding unreferenced blob rows. Startup recovery currently sweeps aged orphans, but integrating immediate cleanup must preserve shared references and deletion retry semantics. Owner confirmation is pending for this lifecycle adjustment and the additional E2E fixture/snapshot files, as requested by the PR author. No changes to those paths or lifecycle behavior have been made for this finding yet. |
db507f5 to
3fab74d
Compare
|
Implemented the approved project-deletion cleanup and remaining CI corrections:
Validation: 104 tests across provenance, project deletion, and content repository suites passed, as did typecheck, source lint, and diff checks. The branch remains three commits with internal planning documents excluded. |
Codex ReviewVerdict: needs changes [P1] Enforce exact-item scope for literature readssrc/main/acp/runtime-composition.ts:205 Impact: When a caller requests Recommendation: Carry the originating Summary: The new exact-item literature scope is not enforced for abstract/PDF reads. Branch and pull-request naming prechecks are valid. |
3fab74d to
06ec556
Compare
|
After tracing both read paths, I am not adopting a previous-search authorization token for the
Added a code comment to make the existing distinction explicit, and replaced the unconditional fallback with explicit handling of the two ungrouped scopes. Introducing a new selection token would change the product's authorization model without closing the claimed escalation, because explicit global Library access is intentional. Library MCP, Reading context, and runtime composition tests passed (39 tests). Separately, an internal boundary test exposed SQLite LIKE semantics in the new project cleanup query: underscores act as wildcards and matching is case-insensitive. Candidate IDs now receive an exact storage-root string check before entering the reference-aware sweep. A regression verifies that deleting project_1 preserves an unrelated orphan under project-1. All 104 provenance/deletion/content tests and Node typecheck passed. On the preceding head, all CI checks passed, including CodeQL, every Ubuntu full-test shard, both Windows E2E shards, and macOS build/E2E. The next push retains the three-commit history. |
Problem
References, downloaded PDFs, citations, and agent reading context need a shared library workflow with explicit review before importing agent suggestions or merging duplicates.
Proposed change
Scope and non-goals
This is the integrated Literature library feature, organized into three commits for core services, UI/research integration, and resilient batch/merge workflows, including accepted PR review corrections. Internal planning documents are excluded from both the PR diff and its commit history. It does not implement a systematic-review screening product or change ROADMAP.
Acceptance criteria and validation
06ec5566, all application CI checks passed: CodeQL, static checks, all Ubuntu full-test shards, Windows core tests and both Windows E2E shards, macOS build/E2E, Linux isolation, and the PR Gate aggregate.git diff --checkpassed. The macOS Provenance visual baseline reflects the approved side-by-side layout; current macOS visual CI passed.Review focus
Review transactional merge validation and attachment/link preservation, migration-ledger adoption, PDF publication/cleanup ordering, Inbox acceptance, and checkpoint/draft persistence across background task transitions.