🦄 new(pipeline): sefaria source acquisition (1.1) - #25
Conversation
Task 1.1: acquire the Jastrow lexicon from its true source. Channel decision (documented in pipeline/README.md): Sefaria's public MongoDB dump is the only channel carrying the lexicon_entry collection; Sefaria-Export and Sefaria-Data lack lexicons and the Words API is per-word only. The script streams the ~2.4 GB dump, extracts just the lexicon collections from the tar in memory, decodes BSON, and emits both Jastrow lexicons (Sefaria also hosts a Jastrow Unabbreviated variant) verbatim as JSONL plus a provenance manifest. Biome override for pipeline/: node modules and sequential awaits are inherent to streaming scripts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
Output of bun pipeline/fetch.ts against the 2026-07-04 dump (etag f499c58f56715a1daada38419be0b615): 32,512 entries, equal to the v1 entry count. All entries carry rid, content, refs, quotes; 8,673 have the alt_headwords v1 dropped; content.morphology and resolved cross-reference links are present. The 'Jastrow Unabbreviated' lexicon in Sefaria's code mapping does not exist in the deployed database, so only the printed dictionary is emitted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
jastrow | 572d134 | Jul 05 2026, 04:14 PM |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Bun-based "Pipeline v2" for acquiring Sefaria's MongoDB dump: streaming tar/BSON utilities, a fetch script producing lexicon JSON/JSONL outputs and a manifest, unit tests, generated data files, and supporting tooling config, documentation, and dependency changes. ChangesPipeline v2 source acquisition
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Main as main() (fetch.ts)
participant Source as DUMP_URL / cache
participant Lib as lib.ts
participant FS as data/source outputs
Main->>Source: download() or load cached BSON
Source-->>Lib: gzip tar stream
Lib->>Lib: extractTargets(reader, targets)
Lib-->>Main: lexicon.bson, lexicon_entry.bson
Main->>Lib: bsonDocuments(lexicon.bson)
Lib-->>Main: registry documents
Main->>FS: write lexicons.json
Main->>Lib: bsonDocuments(lexicon_entry.bson)
Lib-->>Main: entry documents
Main->>FS: write jastrow-dictionary.jsonl
Main->>Lib: sha256(outputs)
Main->>FS: write manifest.json
🚥 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 |
CI runners lack a global bun-types; tsconfig types: ["bun"] needs the package declared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
Maintainer decision amending V4's path examples (recorded in the spec changelog): three top-level roots — app/ (public app), admin/ (maintainer tooling), data/ (data only, no code). v1's data/admin/ mixed code into data/, which broke the semantics of data/** globs in the PR template and review config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@admin/pipeline/fetch.ts`:
- Around line 99-115: `emitRegistry` currently writes `lexicons.json` even when
no documents match `JASTROW_LEXICONS`, which hides schema drift in
`bsonDocuments`/`TARGETS.get(...)`. Add a hard failure in `emitRegistry` after
building `registry` and before `Bun.write` when `registry.length` is zero, using
a clear error that mentions the lexicon registry and the expected `name`-based
match. Keep the existing `progress` success log for non-empty results so the
function only returns a path when a real registry was emitted.
- Around line 48-75: The download flow in download() currently has no timeout or
retry, so a stalled fetch(DUMP_URL) can hang indefinitely. Update the download
logic to use an AbortSignal.timeout() on the fetch call and add a small retry
loop for transient network failures, preserving the existing progress/error
handling and the downstream ChunkReader/extractTargets behavior.
In `@admin/pipeline/lib.test.ts`:
- Around line 106-121: The bsonDocuments test suite only covers the happy path
and misses the truncated BSON error branch. Add a new test alongside the
existing bsonDocuments round-trip test that writes an intentionally shortened
BSON file, drains the async iterator returned by bsonDocuments, and asserts it
throws the truncated BSON document error string. Use the existing bsonDocuments
helper and TEST_DIR path pattern so the new test clearly exercises the
length-prefix guard in lib.ts.
In `@admin/pipeline/lib.ts`:
- Around line 107-141: The tar extraction logic in extractTargets currently
trusts the parsed member size from the header, which can become NaN for
malformed entries and silently desync ChunkReader. Add a validation step right
after parsing the size in extractTargets (before computing padded or calling
reader.consume) that rejects non-finite or negative sizes with a clear error,
and ensure the rest of the extraction path only runs for valid tar sizes.
🪄 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
Run ID: 1ecbdd98-2c2e-4192-9a2a-47e3fa070d44
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.claude/CLAUDE.md.coderabbit.yaml.gitignoreadmin/pipeline/README.mdadmin/pipeline/fetch.tsadmin/pipeline/lib.test.tsadmin/pipeline/lib.tsbiome.jsondata/source/jastrow-dictionary.jsonldata/source/lexicons.jsondata/source/manifest.jsondocs/specs/2026-07-03-v2-overhaul-design.mdpackage.json
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Workers Builds: jastrow
⚠️ CI failures not shown inline (1)
GitHub Check: Workers Builds: jastrow: Workers Builds: jastrow
Conclusion: failure
Build ID: [9ba06845-79d2-433e-8b7b-0bac57c69688](https://dash.cloudflare.com/cf68e0017c53376defadebeecd91fda2/workers/services/view/jastrow/production/builds/9ba06845-79d2-433e-8b7b-0bac57c69688)
Script: [jastrow](https://dash.cloudflare.com/cf68e0017c53376defadebeecd91fda2/workers/services/view/jastrow/production)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
**: General project rules:
- This is a vanilla JS static site PWA. No bundler, no framework, no npm packages at runtime.
- All JS runs in the browser unless under
data/admin/(which uses Bun).- Biome enforces lint and formatting.
- DOMPurify is loaded via CDN with SRI for XSS sanitization.
Files:
package.jsondata/source/manifest.jsondata/source/lexicons.jsondocs/specs/2026-07-03-v2-overhaul-design.mdadmin/pipeline/README.mdadmin/pipeline/lib.test.tsadmin/pipeline/lib.tsadmin/pipeline/fetch.tsbiome.json
🪛 Biome (2.5.1)
admin/pipeline/lib.ts
[error] 6-6: The import specifier bson cannot be resolved: module not found
(lint/correctness/noUnresolvedImports)
admin/pipeline/fetch.ts
[error] 17-17: The import specifier bson cannot be resolved: module not found
(lint/correctness/noUnresolvedImports)
🪛 GitHub Check: Lint
admin/pipeline/lib.test.ts
[notice] 19-19: lint/performance/noAwaitInLoops
Avoid using await inside loops.
admin/pipeline/lib.ts
[notice] 126-126: lint/style/useDestructuring
Use array destructuring instead of accessing array elements by index.
admin/pipeline/fetch.ts
[notice] 154-154: lint/suspicious/noConsole
Don't use console.
🔇 Additional comments (15)
.claude/CLAUDE.md (1)
14-14: LGTM!admin/pipeline/fetch.ts (2)
17-17: 📐 Maintainability & Code QualitySame unresolved
bsonimport asadmin/pipeline/lib.ts.
1-46: LGTM!Also applies to: 77-97, 121-187
admin/pipeline/lib.ts (2)
12-92: LGTM!Also applies to: 94-99, 144-174, 176-176
5-6: 🎯 Functional CorrectnessNo Biome import-resolution issue here.
bsonis already declared indevDependencies, andadmin/pipeline/*.tsis already covered by the Biome override that disablesnoNodejsModules.> Likely an incorrect or invalid review comment.admin/pipeline/lib.test.ts (1)
1-104: LGTM!admin/pipeline/README.md (1)
1-52: LGTM!.coderabbit.yaml (1)
30-53: LGTM! Theadmin/**path instruction update correctly matches theadmin/pipeline/relocation and documents theconsole/sequential-await exceptions this pipeline relies on.docs/specs/2026-07-03-v2-overhaul-design.md (1)
56-56: LGTM! Decision V4 amendment and changelog entry are consistent with theadmin/pipeline/relocation reflected in the code changes.Also applies to: 166-166
data/source/lexicons.json (1)
1-17: LGTM!data/source/manifest.json (1)
1-21: LGTM!Content matches the
emitRegistry/main()producer logic inadmin/pipeline/fetch.ts(paths, sha256 fields, and source provenance shape all line up)..gitignore (1)
13-15: LGTM!biome.json (2)
14-16: LGTM!
284-313: LGTM!Disabling
noNodejsModules/noAwaitInLoopsand allowingsnake_casefor object-literal properties is reasonable for a Bun script that streams raw Mongo BSON fields (e.g.alt_headwords).package.json (1)
7-7: 🎯 Functional CorrectnessConfirm
admin/pipeline/is covered by the Bun/npm-package exception.The active project path instructions state the npm-package/Bun exception applies to
data/admin/("All JS runs in the browser unless underdata/admin/(which uses Bun)"), but this PR's pipeline (and its newbson/@types/bundependencies) lives under the new top-leveladmin/pipeline/path per the layout-decision commit. If.coderabbit.yaml/path instructions haven't been updated in this stack to reflect theadmin/→data/adminlayout move, this dependency addition is technically outside the documented exception.Please confirm the path instructions doc is being updated alongside this layout change (tracked in a later layer of this stack) so the exception scope stays accurate.
As per path instructions, "All JS runs in the browser unless under
data/admin/(which uses Bun)."Also applies to: 17-18
Source: Path instructions
CodeRabbit findings on PR #25: fail fast when the registry or an entry emit comes up empty (schema-drift guard), throw on an unparseable tar member size instead of silently desyncing the reader (consume(NaN) is a no-op), and cover truncated-BSON and bad-size paths in tests. Header parsing extracted to parseTarHeader to keep extractTargets within complexity limits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Brian L. <brian@uniquepixels.xyz>
|
1 participant
What & why
Phase 1, task 1.1 (source acquisition) of the v2 overhaul spec: identify the canonical Sefaria channel for the Jastrow lexicon, script the fetch, and land a fresh source snapshot.
Channel decision (full rationale in
admin/pipeline/README.md): Sefaria's public MongoDB dump (dump_small.tar.gz, refreshed ~daily) is the only channel carrying thelexicon_entrycollection. Sefaria-Export has texts/links only, Sefaria-Data has no Jastrow source, and the Words API is per-word lookup with no bulk export.What landed
admin/pipeline/fetch.ts+lib.ts— streams the ~2.4 GB dump, extracts only the lexicon collections from the tar in memory, decodes BSON, emits verbatim JSONL + provenance manifest (bun run pipeline:fetch, re-runnable;--cachedre-decodes without downloading).admin/pipeline/lib.test.ts— unit tests for the stream/tar/BSON machinery. Also makesbun qapass again (bun testwas failing on zero test files).data/source/— the snapshot: 32,512 entries (equals the v1 entry count), sha256 + ETag + Last-Modified recorded inmanifest.json.admin/pipeline/: node modules and sequential awaits are inherent to streaming scripts.Findings for the divergence audit (1.2)
alt_headwords— the data the v1 extraction dropped.rid,content(incl.morphology),refs,quotes;prev_hw/next_hwchain is complete; cross-references arrive as resolveddata-reflinks.Jastrow Unabbreviated, but the deployed DB has zero entries for it — documented in the README, only the printed dictionary is emitted.Maintainer flags
data/was pruned fromv2in 🧺 chore: remove v1 app for v2 start #23, which contradicts spec Decision V5 ("data/stays until 1.2/1.3 complete"). Nothing is lost (main+ history), but please confirm whether to restore it or amend V5.Checklist
git commit -s) — DCObiome check .passesData changes
data/source/**is a new v2 pipeline artifact, not v1 dictionary data: emitted verbatim bypipeline/fetch.ts(not the admin tool), integrity recorded inmanifest.json. The v1 checklist items (admin tool,validate:data, ≤50 entries) don't apply; validation rules for v2 data arrive with task 2.4.AI assistance
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores