Add rosetta export: DB-only dataset directory (B-0022 E1+E2)#103
Conversation
Implements the B-0022 export track through E2. `rosetta export <dir>` writes a deterministic directory of spreadsheet-friendly TSVs from the runtime SQLite DB alone — no caches, network, or re-extraction — as an audit surface for the artifact rosetta ships. E1 (spine, issue #101): the command, the serialization contract, and `manifest.toml`, proven by `changelog.tsv` + `callouts.tsv`. TSV uses a reversible Postgres-COPY-style escape (`\t`/`\n`/`\r`/`\\` backstopped on every value, whole-field `\N` for SQL NULL distinct from the empty string), stable ordering so a rebuild on the same DB is byte-identical, and a `manifest.toml` carrying `db_meta` provenance, per-file row counts, and honest disclosures. E2 (flat datasets): `properties.tsv` (joined to pages, `section_anchor` resolved via the post-#90 `section_id`), `videos.tsv` (metadata + per-video transcript segment/word/byte counts), `commands.tsv`, and the paired `pages.tsv` (one row per section) / `pages_summary.tsv` (per-page rollup) — two audiences, same core data. Reads only the resolved DB via normal readiness; a column the DB cannot produce is omitted and disclosed (transcript provenance #21, per-version arch #91), never recovered from a source artifact. Products (E3) and per-fragment tables (E4) remain out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds ChangesDataset export
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
Linter is failing check. |
There was a problem hiding this comment.
Pull request overview
Adds a new CLI “export” surface to Rosetta that writes an auditable, deterministic dataset directory from the runtime SQLite database only, producing spreadsheet-friendly TSVs plus a provenance/contract manifest.toml (B-0022 through E2 / issue #101).
Changes:
- Adds
rosetta export <dir>CLI command wiring, including help text and DB readiness gating. - Introduces
src/export.tsimplementing the TSV escaping/NULL contract, manifest emission, and seven flat datasets (changelog/callouts/properties/videos/commands/pages/pages_summary). - Adds a focused test suite proving round-trip escaping, deterministic output, manifest row counts + provenance, and the DB-only hard boundary; updates MANUAL + CHANGELOG.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/mcp.ts |
Adds CLI dispatch + help entry for rosetta export <dir> and runs export after ensureDbReady. |
src/export.ts |
Implements deterministic export pipeline, TSV codec, manifest TOML emission, and dataset readers. |
src/export.test.ts |
Adds contract + determinism + provenance + DB-only-boundary tests for the export spine/E2 datasets. |
MANUAL.md |
Documents the new dataset export command and the emitted file set/serialization contract. |
CHANGELOG.md |
Records the user-visible addition of rosetta export <dir> under [Unreleased]. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@MANUAL.md`:
- Line 287: The documentation claims for rosetta export must distinguish startup
from generation: update MANUAL.md lines 287-287 and CHANGELOG.md lines 27-27 to
state that generation reads only the resolved runtime database, while startup
may download or refresh it; remove any unconditional “no network” promise and
preserve the audit-surface description.
In `@src/export.test.ts`:
- Around line 274-283: Update the “manifest provenance mirrors db_meta” test to
snapshot the existing db_meta values for release_tag, source_commit, and
built_at before inserting test data, then restore those values during cleanup
after assertions complete. Keep the test’s manifest verification unchanged and
ensure restoration runs even if the test fails.
- Around line 307-316: Strengthen the DB-only boundary test around the export.ts
source check instead of only searching for listed spellings. Enforce an explicit
import/dependency allowlist that catches aliases and dynamic imports, or run the
exporter with forbidden filesystem, network, subprocess, and secondary-database
I/O mocked to fail; ensure transitive helpers are covered and only the permitted
stat and Bun.write operations remain usable.
In `@src/export.ts`:
- Around line 257-269: Rename the transcript aggregation map variable from agg
to aggregates throughout the export logic, including its lookup in the
videos.map callback. Rename any rollups identifier or text in the same code path
to roll-ups, or add the terms to the approved spelling dictionary if renaming is
not appropriate.
- Around line 365-371: Update readMeta to remove the broad catch that converts
database errors into null. Let failures from database.prepare or get propagate,
while preserving the existing row?.value ?? null behavior so a genuinely missing
key still returns null.
- Around line 118-122: The encodeFsName implementation must handle Unicode code
points and prevent reserved dot segments. Update its regular expression to use
Unicode-aware matching with the /u flag so astral characters are encoded
distinctly, and ensure exact "." and ".." results are escaped while preserving
the existing encoding for other allowed characters.
- Around line 453-456: In the export flow, update the write ordering around the
dataset loop and manifest write so every dataset TSV is written first and
manifest.toml is written last. Preserve the existing paths and serialization by
moving the manifest Bun.write call after the datasets loop, ensuring readers
only see the manifest after data export completes.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c489b2bf-469e-4eaf-8a76-6654a816b22d
📒 Files selected for processing (5)
CHANGELOG.mdMANUAL.mdsrc/export.test.tssrc/export.tssrc/mcp.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Analyze (javascript-typescript)
⚠️ CI failures not shown inline (2)
GitHub Actions: Test / test: Add rosetta export: DB-only dataset directory (B-0022 E1+E2)
Conclusion: failure
##[group]Run make lint
�[36;1mmake lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
bun run lint
$ biome check src/
Checked 71 files in 233ms. No fixes applied.
bun run lint:md
$ markdownlint-cli2 "**/*.md"
markdownlint-cli2 v0.23.0 (markdownlint v0.41.0)
Finding: **/*.md !CLAUDE.md !AGENTS.md !.github/copilot-instructions.md !.github/instructions/** !.github/prompts/** !.github/skills/** !fixtures/** !tasks/** !briefings/** !node_modules/** !**/node_modules/** !box/** !manual/** !drafts/**
Linting: 10 file(s)
Summary: 0 error(s)
bun run lint:spell
$ cspell lint . --no-progress
src/export.ts:257:9 - Unknown word (agg)
src/export.ts:259:15 - Unknown word (agg)
src/export.ts:263:5 - Unknown word (agg)
src/export.ts:269:15 - Unknown word (agg)
src/export.ts:303:15 - Unknown word (rollups)
CSpell: Files checked: 221, skipped: 1, Issues found: 5 in 1 file.
error: script "lint:spell" exited with code 1
make: *** [Makefile:73: lint] Error 1
##[error]Process completed with exit code 2.
GitHub Actions: Test / 0_test.txt: Add rosetta export: DB-only dataset directory (B-0022 E1+E2)
Conclusion: failure
##[group]Run make lint
�[36;1mmake lint�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
bun run lint
$ biome check src/
Checked 71 files in 233ms. No fixes applied.
bun run lint:md
$ markdownlint-cli2 "**/*.md"
markdownlint-cli2 v0.23.0 (markdownlint v0.41.0)
Finding: **/*.md !CLAUDE.md !AGENTS.md !.github/copilot-instructions.md !.github/instructions/** !.github/prompts/** !.github/skills/** !fixtures/** !tasks/** !briefings/** !node_modules/** !**/node_modules/** !box/** !manual/** !drafts/**
Linting: 10 file(s)
Summary: 0 error(s)
bun run lint:spell
$ cspell lint . --no-progress
src/export.ts:257:9 - Unknown word (agg)
src/export.ts:259:15 - Unknown word (agg)
src/export.ts:263:5 - Unknown word (agg)
src/export.ts:269:15 - Unknown word (agg)
src/export.ts:303:15 - Unknown word (rollups)
CSpell: Files checked: 221, skipped: 1, Issues found: 5 in 1 file.
error: script "lint:spell" exited with code 1
make: *** [Makefile:73: lint] Error 1
##[error]Process completed with exit code 2.
🧰 Additional context used
📓 Path-based instructions (7)
*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep this Copilot instructions file short; put substantive rules in narrow instruction files under
.github/instructions/*.instructions.md.
Files:
CHANGELOG.mdMANUAL.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Record user-visible shipped changes in
CHANGELOG.mdunder[Unreleased]or release sections.
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer each document's canonical home and do not duplicate operational detail, schema blocks, or long rule lists in the routing index.
Files:
CHANGELOG.mdMANUAL.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Bun and TypeScript, including Bun-native runtime APIs such as
bun:sqlite,Bun.serve, andbunx.Rosetta uses Bun and TypeScript; prefer
bunandbun testrather than Node/npm-oriented substitutes.
Files:
src/mcp.tssrc/export.tssrc/export.test.ts
src/{query,mcp,browse}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Route MCP, query, classifier, TUI, and canonicalizer behavior through shared core code, usually
src/query.ts; keepsrc/mcp.tsandsrc/browse.tsthin adapters.
Files:
src/mcp.ts
{README.md,src/setup.ts,MANUAL.md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep public client setup snippets aligned across
README.md,src/setup.ts, andMANUAL.md, including the Codex commandcodex mcp add rosetta -- bunx@tikoci/rosetta``.
Files:
MANUAL.md
MANUAL.md
📄 CodeRabbit inference engine (CLAUDE.md)
Put installation, operations, release/re-extraction procedures, and schema reference in
MANUAL.md.
Files:
MANUAL.md
🪛 ast-grep (0.44.1)
src/export.test.ts
[warning] 205-205: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(name = "${file}"[\\s\\S]*?rows = ${dataRows}\\b)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🪛 GitHub Actions: Test / 0_test.txt
src/export.ts
[error] 257-257: cspell lint: Unknown word "agg".
[error] 259-259: cspell lint: Unknown word "agg".
[error] 263-263: cspell lint: Unknown word "agg".
[error] 269-269: cspell lint: Unknown word "agg".
[error] 303-303: cspell lint: Unknown word "rollups".
🪛 GitHub Actions: Test / test
src/export.ts
[error] 257-257: CSpell lint failed: Unknown word (agg)
[error] 259-259: CSpell lint failed: Unknown word (agg)
[error] 263-263: CSpell lint failed: Unknown word (agg)
[error] 269-269: CSpell lint failed: Unknown word (agg)
[error] 303-303: CSpell lint failed: Unknown word (rollups)
🔇 Additional comments (4)
src/export.ts (1)
1-117: LGTM!Also applies to: 123-256, 270-298, 305-364, 372-452, 457-459
src/mcp.ts (1)
184-184: LGTM!Also applies to: 222-236
MANUAL.md (1)
30-30: LGTM!Also applies to: 285-286, 289-315
src/export.test.ts (1)
1-273: LGTM!Also applies to: 286-305
…ording, boundary test Review fixes from CodeRabbit + Copilot on PR #103, plus the CI-blocking cspell: - cspell: rename `agg` → `aggregates`, reword "rollups" → "rollup" (CI green). - runExport: create the target dir explicitly with mkdirSync instead of leaning on Bun.write's implicit parent creation; drop the now-stale comment. - encodeFsName: add the `u` flag so astral characters (emoji) encode distinctly instead of collapsing to shared surrogate-replacement bytes, and escape the reserved "." / ".." directory names. New test covers both. - videos docstring: stop asserting "every video has at least one segment" — the schema doesn't enforce it; the aggregate already falls back to honest zeros. - MANUAL.md / CHANGELOG.md: distinguish network-free dataset *generation* from startup DB readiness, which may download/refresh like every command. - export.test.ts: snapshot/restore db_meta in the provenance test so fake values don't leak into the shared DB singleton; add an import-specifier allowlist to the DB-only boundary test to catch aliased/new/dynamic I/O imports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two more CodeRabbit findings on PR #103: - readMeta: narrow the catch to a genuinely absent db_meta table (pre-v5 / non-rosetta DB); let corruption/locking/query errors propagate instead of disguising them as "(unstamped)" provenance (crash-early house style). - runExport: write the dataset TSVs first and manifest.toml last, so a failed run can't leave a manifest naming data files that were never fully written. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…itively true Addresses the transitive-import boundary hole (external GPT-5.6 review, finding #3): export.ts imported compareVersions from query.ts, which imports db.ts and opens the global database at import time — so importing export.ts under an invalid DB_PATH threw, and the "reads only the passed handle" claim was only true at runtime, not for the module graph. - New src/version-compare.ts holds compareVersions as a dependency-free module (no bun:sqlite / db.ts / paths.ts). query.ts imports and re-exports it, so `from "./query.ts"` callers are unchanged; export.ts imports it directly. - Boundary test strengthened: the import allowlist now points at ./version-compare.ts and additionally asserts that module imports nothing (closing the transitive closure), plus a behavioral test that importing export.ts under a nonexistent DB_PATH opens no database. Documents the known compareVersions prerelease-number limitation (7.24beta1 vs 7.24beta2 compare equal) inline; tracked for a shared-comparator fix in the E1/E2 follow-up issue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ose it in manifest Makes the tracking reference precise and keeps the manifest's ordering claim honest: changelog.tsv is version-ordered but same-base prereleases fall back to sort_order until the shared comparator learns prerelease numbers (#104). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
External review (GPT-5.6)
Claude's dispositionRan an additional review; here's how each finding was handled.
Follow-up issue: #104. Full suite 874 pass / 0 fail; typecheck, biome, cspell, markdownlint clean. |
Implements the B-0022 export track through E2 — a
rosetta export <dir>command that writes a deterministic directory of spreadsheet-friendly TSVs from the runtime SQLite database alone (no caches, network, or re-extraction). It is an audit surface for the artifact rosetta actually ships. Briefing:briefings/B-0022-runtime-sqlite-dataset-exports.md.bunx @tikoci/rosetta export /tmp/rosetta-datasetsE1 — the spine (Closes #101)
The command, the serialization contract, and
manifest.toml, proven by two datasets:\\/\t/\n/\rbackstopped on every value, whole-field\Nfor SQL NULL as a fact distinct from the empty string. Reversible, keeps one-line-per-record,awk -F '\t'-parseable.manifest.tomlwithdb_metaprovenance (release tag, schema version, source commit, built-at — no wall-clock value), per-file row counts, the full contract, and honest disclosures. A column the DB cannot produce is omitted and disclosed, never recovered from a source artifact.countWords(matches the extractor),utf8Bytes, andencodeFsName.E2 — flat datasets
properties.tsv— joined topages;section_anchorresolved via the post-Properties: 165 rows silently destroyed on insert; section identity stored as heading text (properties + callouts) #90section_id(stored heading text and resolved anchor sit side by side).videos.tsv— metadata plus per-video transcript segment / word / UTF-8-byte counts.commands.tsv— the command glossary, direct.pages.tsv(one row per section) /pages_summary.tsv(per-page rollup) — same core data for two audiences (spreadsheet vs. GitHub-readable), per the split decided in export E1 — spine: command, TSV contract, manifest.toml (+ changelog.tsv, callouts.tsv as proof) #101.Disclosures (what the DB can't say)
command_versionsis arch-blind — command_versions is arch-blind: dual-arch versions are last-writer-wins between x86 and arm64 #91.Out of scope
Products (
products/**, E3) and per-fragment table files (pages/<slug>/**, E4); CI wiring / Pages publication; any MCP/TUI surface change.Verification
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
rosetta export <dir>to export a deterministic, spreadsheet-friendly dataset from the local database.manifest.tomland seven TSV files with stable ordering, reversible escaping, and explicitNULLhandling.Documentation