feat: unify archive metadata storage#94
Conversation
Summary by CodeRabbit
WalkthroughThis PR migrates archive/book metadata persistence from a file-based Sequence Diagram(s)sequenceDiagram
participant CLI
participant runArchiveCommand
participant writeArchiveRoot
participant readArchivePage
CLI->>runArchiveCommand: get objectId
runArchiveCommand->>runArchiveCommand: isArchiveRootGet(args)
alt targets archive root
runArchiveCommand->>writeArchiveRoot: writeArchiveRoot(args)
writeArchiveRoot-->>CLI: JSON {uri} or "<archive>\n"
else targets specific page
runArchiveCommand->>readArchivePage: readArchivePage(uri)
readArchivePage-->>runArchiveCommand: page data
runArchiveCommand-->>CLI: writePage(data)
end
sequenceDiagram
participant Document
participant MetadataStore
Document->>MetadataStore: getMap("")
MetadataStore-->>Document: metadata map (or empty)
Document->>Document: parse via bookMetaSchema
Document->>MetadataStore: replaceMap({kind: Archive, objectPath: ""}, meta)
Compact metadata
Related issues: None specified Related PRs: None specified Suggested labels: refactor, cli, documentation, tests Suggested reviewers: None specified 🐰 A rabbit dug where book-meta lay, 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/cli/object-metadata.test.ts (1)
170-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding execution-level coverage for
delete/clearoutput formatting.This suite verifies
get/puttext vs. JSON rendering end-to-end, but fordelete/clearonly CLI argument parsing is tested (notrunObjectMetadataCommanditself producing text/JSON output). GivenwriteMetadataMapis now invoked identically after delete/clear, a short execution test would close this gap.🤖 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 `@test/cli/object-metadata.test.ts` around lines 170 - 206, Add execution-level test coverage for the object-metadata delete/clear paths in runObjectMetadataCommand, since the current test only checks parseCLIArguments. Extend the existing object-metadata CLI tests to assert the actual output produced after delete/clear for both text and JSON modes, similar to the get/put coverage, and verify that writeMetadataMap is exercised consistently for both actions.src/cli/object-metadata.ts (1)
161-179: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winKeep metadata key order consistent across text and JSON output
formatMetadataTextpreserves insertion order, whileformatCLIJSONapplies the CLI key-priority ordering. That makes fields liketitleordescriptionmove when toggling--json; use the same ordering in both branches.🤖 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 `@src/cli/object-metadata.ts` around lines 161 - 179, The metadata text formatter currently iterates Object.entries(map) directly, so its key order can differ from the CLI JSON output ordering. Update formatMetadataText to use the same key-ordering logic as formatCLIJSON, reusing the shared ordering helper or extracting one if needed, so fields like title and description stay in a consistent position across both output modes.
🤖 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.
Nitpick comments:
In `@src/cli/object-metadata.ts`:
- Around line 161-179: The metadata text formatter currently iterates
Object.entries(map) directly, so its key order can differ from the CLI JSON
output ordering. Update formatMetadataText to use the same key-ordering logic as
formatCLIJSON, reusing the shared ordering helper or extracting one if needed,
so fields like title and description stay in a consistent position across both
output modes.
In `@test/cli/object-metadata.test.ts`:
- Around line 170-206: Add execution-level test coverage for the object-metadata
delete/clear paths in runObjectMetadataCommand, since the current test only
checks parseCLIArguments. Extend the existing object-metadata CLI tests to
assert the actual output produced after delete/clear for both text and JSON
modes, similar to the get/put coverage, and verify that writeMetadataMap is
exercised consistently for both actions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2e24300-2cca-4d4e-8154-6e44a40b04c1
📒 Files selected for processing (24)
data/help/commands/archive/get.jinjadata/help/commands/maintenance/meta.jinjasrc/archive/query/archive-view.tssrc/cli/archive-maintenance.tssrc/cli/archive.tssrc/cli/args.tssrc/cli/help.tssrc/cli/object-metadata.tssrc/document/document.tssrc/facade/import.tssrc/legacy-sdpub/upgrade.tssrc/output/epub/book.tssrc/wikg/archive.tstest/cli/archive-maintenance.test.tstest/cli/archive.test.tstest/cli/object-metadata.test.tstest/document/directory-document.test.tstest/facade/digest.test.tstest/facade/import.test.tstest/gc/gc.test.tstest/legacy-sdpub/upgrade.test.tstest/output/epub-book.test.tstest/wikg/archive.test.tstest/wikg/spine-digest-file.test.ts
💤 Files with no reviewable changes (3)
- src/wikg/archive.ts
- src/legacy-sdpub/upgrade.ts
- test/legacy-sdpub/upgrade.test.ts
Summary
Tests