Remove the scribe package by tjcouch-sil · Pull Request #2 · paranext/scripture-editors

tjcouch-sil · 2026-08-31T23:09:46Z

Removes packages/scribe and the wiring it left behind.

Why

Nothing in Platform.Bible consumes @eten-tech-foundation/scribe-editor. This repo's own CLAUDE.md already described the package as "Not currently maintained — do NOT use it as a reference for existing patterns or conventions". Carrying it means building, typechecking, and testing a package no consumer depends on.

Kept as its own PR so the diff against eten-tech-foundation/scripture-editors stays easy to reason about: the adoption PR (#1) touches documentation and workflows, this one touches code.

What goes

packages/scribe/
tsconfig.json project reference, eslint.config.mjs ignore entry
README demo-table and package-table rows, their npm badges and footnotes, and the mermaid graph nodes
the CLAUDE.md guidance bullet
the pnpm-lock.yaml workspace entry (lockfile regenerated)

One code comment changes: libs/shared/src/nodes/usj/index.ts used scribe's NoteEditor as its worked example of a non-react host. The point it makes — that a non-react host builds straight from usjBaseNodes, so every USJ-shaped editor needs AttributeRunNode registered — still stands without the example.

History is untouched. Scribe's commits and its 8 scribe_v* release tags are still in this repo; only the working tree loses the directory.

Rebased onto current `main` (`863a206`)

Main had picked up more scribe since the last rebase; this pass removes all of it:

Conflicts (modify/delete): Editor.tsx, the three *-usj*.adaptor.ts adaptors, and styles/index.css under packages/scribe, all modified on main by PT-4370: Settled positions across the public position API #11 and Fix the editor context menu and \id-line editing #14. Resolved by deleting them.
A new file: packages/scribe/src/editor/adaptors/noteCallerMerge.test.ts, added by PT-4370: Settled positions across the public position API #11. A rebased deletion commit can't see a file added after it, so this one is deleted explicitly.
Changelogs (new commit): both CHANGELOG.md preambles named Publish Scribe Package as the only publish workflow. That workflow went away in Adopt this repository into the paranext organization #1, so they now point at Publish. The utilities usjToUsxString entry also carried a caveat that the Scribe editor did not carry the USJ version through, and that caveat is dropped.
Committed dist (new commit): the usjBaseNodes comment this PR rewrites is bundled into packages/platform/dist/index.js, so the dist is regenerated (a 3→2 line comment change). Without it, the Check the committed build output is current step would fail on this PR's own change.

Review follow-ups (one commit): pnpm-workspace.yaml no longer lists @tailwindcss/oxide in onlyBuiltDependencies. Nothing installs it once scribe's Tailwind toolchain is gone, and the lockfile doesn't change. Two comments now name real callers: UsjNodesMenuPlugin's doc comment, and the usjBaseNodes note on why AttributeRunNode is in the base list. The changelog intros now say "released" throughout. The dist is regenerated again to match.

pnpm-lock.yaml merged cleanly, and pnpm install --lockfile-only leaves it unchanged.

What remains: a whole-word scribe grep of the tree now finds only Bible text ("scribes", "Ascribe"), plus docs/transferring-work-from-eten-tech-foundation.md listing the historical scribe_v* tags. Those tags still exist, so that line stays.

The second commit (docs: drop the remaining scribe references) is unchanged from the previous round: the CLAUDE.md nx dev scribe blocks, the architecture diagram, and UsjNodesMenuPlugin's doc comment.

Verification

Run on the rebased branch: nx format:check, then run-many for lint (warnings only), typecheck, test, and build across all nine projects, all clean. Then scripts/rebuild-committed-output.mjs followed by scripts/verify-committed-dist.mjs, which reports "Committed build output is current. ✓".

Merge order

#1 and #9 have both merged, so nothing blocks this. The last CI failure was the stale-dist inheritance #9 fixed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VCWPCf35QD5sTkFghsTfUX

This change is

`homepage`, `repository`, and `bugs` still named eten-tech-foundation, so everything npm derives from them — the repository link on npmjs.com, `npm repo`, and the "report an issue" link — sent people to the repository this one replaces. The package names and their `@eten-tech-foundation` scope are deliberately unchanged: publishing continues under that scope, and renaming would break every consumer. Only the location of the source moves. `packages/scribe` is left alone; #2 removes that package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017jvJ2eYTfxJaBkKoQbVeiW

rolfheij-sil · 2026-09-07T10:48:04Z

🤖 Claude: Removal is complete against everything checked (one review pass): packages/scribe gone (67 files), tsconfig.json reference, eslint.config.mjs ignore, CLAUDE.md bullet, README rows/badges/mermaid node, pnpm-lock.yaml regenerated, pnpm-workspace.yaml correctly untouched (glob). The libs/shared/src/nodes/usj/index.ts comment edit keeps the real point. Good call re-running the typecheck-before-build control on unmodified main.

One thing to know for the merge: this is a sibling of #1 (same base), and git merge-tree shows a real conflict in README.md and pnpm-lock.yaml whichever lands second — plan a manual resolve + lock regeneration rather than expecting a clean rebase.

* docs: adopt this repository into the paranext organization This repo is now maintained at paranext/scripture-editors and is the source of truth for the editor packages. It carries the full history of eten-tech-foundation/scripture-editors, its MIT license, and its attribution, but it is deliberately not a GitHub fork: while a repository is a fork, GitHub always defaults new pull requests to the parent and offers no setting to change that. Documents what that means in practice: - Where this repository lives, and its relationship to paranext-core — which builds these packages from source and stages them rather than installing them from npm, so this repo's package.json files are authoritative for its own dependencies. - Why the platform-yalc branch still exists. The name is historical, but its coordination role — letting a breaking change land on main before build servers pick it up — is not. - How to set up the eten-tech-foundation remote, pull from it, and contribute back, so that option survives even though we do not track it. - How to move branches, tags, and open PRs across from the other repository, since this one was seeded with a curated ref set rather than a full mirror. Replaces the npm-publishing machinery with a release process modeled on paranext-core's. Consumers pin a revision of this repository, not a published version, so a release is a repository tag and one tag pins both packages. Adds a bump-versions script, workflow, and composite action, plus a publish workflow that verifies the requested version matches the manifest before tagging. Removes the scribe publish workflow and the tag-triggered npm publish and auto-bump jobs. Drops the Nx Cloud references. There is no account for this organization, and the caching does not earn standing one up. Adds a SIL Global and United Bible Societies copyright line alongside ETEN Tech Foundation's. The license stays MIT, which is what keeps contributing back possible. * style: apply this repo's prettier formatting to the added files * feat: verify paranext-core's lockfile stays in sync on platform-yalc pushes paranext-core's package-lock.json records these packages' dependency closure, so a dependency added, removed, or re-ranged here breaks every core build the moment platform-yalc moves — core's `npm ci` refuses to run until its lockfile is refreshed. That refresh is a one-command PR in core, but nothing reminded the person moving the branch to open it. Adds a workflow that runs on every push to platform-yalc: it recomputes what core's lockfile should record from this repo's manifests (applying the same transforms core's staging applies — devDependencies dropped, workspace: specifiers rewritten to the sibling staged folder, the package list read from core's own dev-packages.json so the repos cannot drift) and passes when core's main matches. When it does not, it scans open core PRs touching package-lock.json for one that brings it in sync — the refresh being in flight is fine — and otherwise fails with the exact commands to fix it. Version-only bumps pass untouched; npm ci does not check the version of a file: dependency, so releases need no core PR. The README's platform-yalc section documents the same flow for humans. Verified against live data: the in-sync path against the core PR branch, and a fabricated dependency change exercising the mismatch diff, the open-PR scan (152 open PRs, paginated), and the failure instructions. * feat: move-platform-yalc script — rebase, verify consumer sync, push The workflow catches a missing core lockfile PR after the push; this catches it before. Operates on a temporary branch from origin/platform-yalc (never the often-stale local branch), restores the original checkout on exit, aborts cleanly on rebase conflicts, and runs the same verify-consumer-lockfile-sync check the CI workflow runs. --dry-run stops short of pushing; --skip-verify is the documented emergency hatch. * build: commit the published packages' dist paranext-core consumes these packages by copying them out of a checkout rather than installing them from a registry, so until now every core developer needed this repo's toolchain — pnpm, nx, a full vite build and api-extractor run — during `npm install`, just to run Platform.Bible. That build is also where a whole class of install failures lives. Committing dist reduces the consumer's work to a copy. Only people changing the editor build it. The obligation this creates is that dist can go stale against src, and a stale dist is invisible in review: the source diff looks right while consumers get old code. CI now rebuilds and fails if the committed output differs (scripts/verify-committed-dist.mjs). Every published artifact is byte-deterministic — verified by rebuilding and comparing hashes — so the check is exact rather than advisory. Also stops packing dist/*.tsbuildinfo. That is TypeScript's incremental cache, the one non-deterministic file in the build, and it was being copied into consumers' staged copies as though it were a shipped artifact. * build: print the actual diff when the committed dist check fails A --stat summary cannot distinguish a genuine source change from a build that is not reproducible across machines, which is exactly the question a CI failure here raises. * build: verify the committed dist against a real build, not the nx cache nx declares `dist` a cached target output, and earlier steps in this job populate that cache, so a cache hit restores nx's copy over the committed one — the check would then be grading the cache instead of the source. It also explains an earlier spurious failure of this step: the first run after committing dist rebuilt over it incrementally. * fix: commit api-extractor's rolled-up dist, not tsc's intermediate output The first dist commit captured a mid-build state: tsc's per-file declarations under dist/converters/ plus an index.d.ts that merely re-exported them. api-extractor consumes those and emits the rolled-up API surface as index.d.ts, then removes them — so which files exist depended on which nx targets last ran, and CI's freshness check failed against a local build for exactly that reason. Commits what a canonical build leaves: the rolled-up index.d.ts and the bundles. The intermediate per-file declarations are gitignored, since whether they are present is not meaningful and nothing references them — `types` resolves to dist/index.d.ts. * fix: ignore tsc's declaration map alongside its per-file declarations Same class of intermediate artifact: api-extractor rolls the declarations into dist/index.d.ts and the map beside them becomes meaningless, but a plain `build` leaves it, so its presence depends on which targets last ran. CI caught it as an untracked file. * docs: point the published packages' metadata at this repository `homepage`, `repository`, and `bugs` still named eten-tech-foundation, so everything npm derives from them — the repository link on npmjs.com, `npm repo`, and the "report an issue" link — sent people to the repository this one replaces. The package names and their `@eten-tech-foundation` scope are deliberately unchanged: publishing continues under that scope, and renaming would break every consumer. Only the location of the source moves. `packages/scribe` is left alone; #2 removes that package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017jvJ2eYTfxJaBkKoQbVeiW * fix: close the gaps review found in the release and lockfile guards `publish.yml` created the release tag after nothing but a build. The workflow it replaced gated every publish on `needs: test`, and the committed-dist check lives in `test.yml`, which only triggers on `main` — so a release cut from any other branch was verified by nothing, and a tag cannot be un-cut. Run the same checks `main` gets, all of them before the release step. The bump step after the release could never have worked: `actions/checkout` cleans the workspace, which takes `node_modules` with it, and the composite action then runs `tsx`. By that point the tag exists and `bump-versions` is not idempotent, so the failure has to be unpicked by hand. Reinstall first. `verify-consumer-lockfile-sync.mjs` compared `dependencies` and `peerDependencies` only. `peerDependenciesMeta` is what marks a peer optional, so deleting an entry makes that peer required and npm pulls it into the closure: verified against a staged `file:` dependency, making `yjs` required this way fails every core `npm ci` with "Missing: yjs from lock file" while both dependency lists read identically. Compare it too, structurally, since its entries are objects. That guard also spent one API request per open core PR — 154 today — to decide which touch the lockfile, which exhausts the anonymous rate limit it runs under locally. Look at recently-updated PRs first and walk the rest only when the alternative is failing. A file list at the page limit now counts as a candidate rather than being read as "no lockfile here", a deleted head fork is skipped instead of throwing, the raw.githubusercontent reads authenticate like the API reads, and an empty `CORE_BRANCH` falls back to `main` instead of building a malformed URL. `verify-committed-dist.mjs` capped its failure output at 200 lines, which bounds nothing when `index.js.map` is three megabytes on one line. It also now records the blind spot it cannot cover on its own — `git status` sees no ignored path — and points at #5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * chore: finish the adoption cleanup review turned up `nx.json` still carried ETEN's Nx Cloud access token and workspace id, disabled only by an underscore on each key. Nx Cloud is off for this repo and the credentials are not ours; remove them. Its `sharedGlobals` also named `.github/workflows/ci.yml`, which this repo has never had, so no workflow change has ever invalidated a cached target — point it at `test.yml`. paranext-core consumes these packages by staging a built copy out of a checkout, so `yalc` has no remaining role: drop the dependency and the `devpub` scripts, and replace both "Develop in App" sections with the flow that works now (`npm run build:editor` in core). Badges pointed at the old organization and at `test-publish.yml`, which no longer exists, and showed an npm version for packages that are not published to npm — a repository tag is what pins them. Two docs named that workflow too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * ci: verify release branches the way paranext-core does `test.yml` ran on `main` only, so a release cut from `release-prep` or a `hotfix-*` branch had no formatting, lint, typecheck or test run against it anywhere. paranext-core covers those branches in its own test workflow and therefore runs no checks at publish time; this repo had the triggers narrower and the gap sat in the middle. Widen the triggers instead of duplicating the checks into `publish.yml`. The committed dist stays checked at publish, because core has no equivalent of it: a tag is where consumers copy their editor from, a tag cannot be un-cut, and a stale dist inside one looks like nothing at all. `extract-api` is the build, so it replaces the separate build step rather than adding to it. The post-release checkout now uses `clean: false`, matching core, rather than reinstalling — and only runs when the bump ref actually differs from the branch already checked out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * perf: only consider recently-updated core PRs as in-flight lockfile refreshes Deciding whether one open core PR touches `package-lock.json` costs an API request, and core carries 150+ open at a time. Run locally by `move-platform-yalc` there is no token, so the scan exhausted the 60/hour unauthenticated limit and failed the check exactly when it mattered. A lockfile refresh for the change being pushed is worked on alongside it, so PRs untouched for two weeks are not it. `move-platform-yalc --skip-verify` remains the escape hatch if that ever stops being true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * docs: point at both ways out when the lockfile check fails The failure told you to refresh core's lockfile and open a PR, which is right when the refresh does not exist yet. It said nothing about the case where it does and this did not find it — the PR has gone untouched past the search window, or the two changes are being landed together on purpose — leaving the escape hatch discoverable only by reading the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * perf: narrow the in-flight PR search to a week, and say what it looked at Core carries 155 open PRs, nearly all stale. Measured against it, a 7-day window selects 51 and a 14-day window 60, against an unauthenticated budget of 60 requests an hour — 7 days fits with room for the listing calls, 14 does not. The failure line also reported three bare numbers with no way to tell what they counted, which read as though most PRs had been skipped. Say plainly how many were examined, how many exist, and how many of those examined touch the lockfile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqqozB7YdhQFin63R2gBpL * fix: harden the release workflows and drop the last devpub references - `bump-versions-action` and `publish.yml`'s version check spliced `workflow_dispatch` inputs straight into bash via `${{ }}`. They now arrive through `env` and quoted expansions, so what someone types cannot become shell. - `COMPARED_SECTIONS` omitted `optionalDependencies`. npm records it for a staged `file:` package and pulls its entries into the closure, so adding one would pass this check and then fail every paranext-core `npm ci` — the breakage the check exists to catch. - `CLAUDE.md` and `docs/dependency-updates.md` still routed maintainers through `nx devpub`, which this change removes from both published packages. The packing gate is now the `npm pack --dry-run` paranext-core's staging step actually calls, so a break there is a break here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: make move-platform-yalc fail safely and stop writing to a guessable temp path The verify-script snapshot went to `$TMPDIR/verify-consumer-lockfile-sync-<pid>.mjs` and was then executed. Pids are small and enumerable, `copyFileSync` follows an existing symlink, and there is a window between the write and the run — so on a shared machine another local account could have this script execute their code inside a checkout that is about to be force-pushed. It now writes into a 0700 `mkdtemp` directory and removes the whole directory afterward. `git rebase --abort` also ran unguarded inside the rebase's own catch. A rebase that fails without starting — an unreachable `origin/main`, or a leftover rebase directory — makes the abort fail too, and that throw escaped over the conflict message: the operator saw `Command failed: git rebase --abort` instead of the real cause, and was told the branch was back at origin's state when the reset above had already moved it. The abort now has its own try, and the fallback reports the original rebase error and where the checkout actually is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: make the lockfile check read fresh, scope it, and let publish re-run - `verify-consumer-lockfile-sync` read core's files through raw.githubusercontent, which serves `max-age=300`. The recovery this check prescribes — land the core lockfile refresh, then re-run — could therefore still be answered from the pre-merge blob, while the open-PR fallback no longer matched because that PR had just closed, leaving `--skip-verify` as the only way out of a problem already fixed. The contents API serves the same bytes at `max-age=60`; the raw media type carries a lockfile past the 1MB cap on the JSON representation. - It also folded every repo in core's `dev-packages.json` into one list and resolved each `packagePath` against this checkout. A second dev repo would make it read manifests that are not here. It now selects this repo's entry by `folder`, which is stable across the org move in a way `cloneUrl` is not — matching on the URL would have replaced the accurate "that core branch predates staged-dependency consumption" message with a wrong one for the whole transitional window. - `bump-versions` ran `git checkout -b` and `git commit` unconditionally, so a re-run (branch exists, exit 128) or a publish whose post-release version equals the released one (nothing staged, exit 1) failed after `publish.yml` had already cut the tag — which cannot be un-cut. It now uses `-B` and skips the commit when the manifests already say the new version, so the step the workflow's own comment called "unpicked by hand" is re-runnable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * ci: build on the Volta-pinned Node instead of hardcoded 20 Three workflows set up Node 20 while `package.json`'s Volta pin — the version every developer runs — is 22.22.0. That gap matters most in `publish.yml`, which rebuilds `packages/*/dist` and compares it against the committed copy before cutting the release tag consumers copy their editor out of: any Node-sensitive difference in that output either fails the comparison with a diff nobody can reproduce locally, or, where the difference is invisible to it, ships bytes no developer ever ran. `node-version-file: 'package.json'` makes setup-node read the Volta pin, so the three stay in step with local development by construction. The repo declares no `engines`, so the pin is what it resolves. `test.yml`'s matrix loses its node dimension, which only ever held one value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * docs: say when a release tag also needs a paranext-core lockfile refresh Core stages these packages inside its own tree and depends on them with `file:` specifiers, so its `npm ci` validates the staged manifests' dependency sections against `package-lock.json` and aborts on any difference. Pinning a tag whose dependency lists differ from the revision core's lockfile was generated against therefore breaks every core build, and any paratext-10-studio release pinned to that tag, with no fix available from either of those repos. Nothing said so. Release step 6 now does, and notes the case that needs nothing: a release that only moves the packages' own version numbers, which `npm ci` does not check for a `file:` dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * ci: test platform-yalc, and restore it when the lockfile check fails `platform-yalc` is the revision paranext-core stages the editor from, and staging is a copy of the committed `dist/` — but no workflow ran on that branch. A commit landing there without a rebuild, whether a direct fix or a rebase conflict resolved in `src/`, shipped stale bytes to every consumer with nothing checking. It now runs the same job `main` does; the dist check is `run-many`, not `affected`, so it really executes there. `move-platform-yalc` did its reset and rebase before the consumer lockfile check, and nothing put the branch back when that check failed — for a real mismatch or equally for a GitHub rate-limit. The branch was left diverged from origin with nothing saying so, and the next run's "commits origin does not have" guard then described this script's own rebase as the operator's stray work. It now resets back to origin's state and says the rerun is safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * docs: name paranext-core as the only place the editor revision is pinned paratext-10-studio's `productInfo.json` no longer states a revision for `devPackageRepos` — it follows paranext-core's `dev-packages.json` — so the release steps no longer need consumers changed in two places at once, and the warning about the two disagreeing no longer describes anything that can happen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: name the rate limit, disclose the utilities version, correct release steps From rolfheij-sil's review of #1: - `move-platform-yalc` is the documented local command, and `GITHUB_TOKEN` is set only inside Actions — so the anonymous 60-requests-an-hour budget this check is sized against is the one developers actually run under. Exhausting it arrived as a bare `GET … -> 403 Forbidden`. It now says what happened and how to fix it. - A repo tag pins both packages but its name carries only platform-editor's version, so which scripture-utilities a consumer gets was invisible from the releases page. `publish.yml` now reads it and puts both in the release body. - The release steps said to merge the `bump-versions-<version>` branch. The target branches are protected and the workflow only pushes the branch, so that is opening a PR from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: recognise every rate-limit shape, and say so in the README The 403 handling added earlier covered only half of what it needed to. GitHub answers a rate limit with 403 **or** 429: the primary limit zeroes `x-ratelimit-remaining`, a secondary limit sends `retry-after` instead, and the 429 form of both was not matched at all. The `!GITHUB_TOKEN` fallback also labelled every unauthenticated 403 a rate limit, including a genuine permissions failure — it now says so hedged rather than confidently wrong, since a token is the right advice either way. Verified across nine response shapes: both statuses with each header, a permissions 403 with a token and requests remaining, 404, 500, and a bare 403 with and without a token in the environment. The README half of that review point was missing entirely. The section on moving `platform-yalc` now says the check spends tens of requests against the 60-an-hour anonymous budget, that Actions sets `GITHUB_TOKEN` but a local run does not, and gives the one line that fixes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * docs: send the platform-yalc section to the script, not a hand rebase The section a reader reaches first when asking how to move this branch gave the raw `git rebase && git push --force-with-lease` sequence, and only mentioned `move-platform-yalc` further down under dependency changes — so the guards were documented somewhere other than where the question gets asked. It now leads with the script and what it refuses (another branch, a dirty tree, commits only on your machine, a lockfile that no longer matches), and links to the existing section for the check, its flags, and the token. The hand sequence stays only as what skipping it costs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * docs: say staging copies the committed dist rather than building paranext-core builds these packages only when the pinned revision has no committed `dist/` to copy. Saying it "builds them from source" describes the fallback as if it were the path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: pass when core's branch records no staged-dependency contract `verify-consumer-lockfile-sync` threw when core's `dev-packages.json` had no `packagePath`/`stagingFolder`, with a message saying "there is no lockfile contract to verify against it" - a no-op reported as a failure. That is core's `main` today, and stays so until the staged-dependency change lands there, so every push to `platform-yalc` fails and `move-platform-yalc` refuses to push. The only way through is `--skip-verify`, which also disables the checks that DO apply once it lands. It now logs the reason and passes. Deliberately not extended to core PRs: the `packagePath` and `stagingFolder` read from the branch are what locate the manifests and name the lock entries, so a run that cannot read them cannot compute what to compare against a PR either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDvvB9RpsxEE49f75ZtB6 * fix: check the committed API reports, not just the dist `etc/<package>.api.md` is API Extractor's report on the public type surface. It is committed so that a public API change — an added export, a changed signature, a removed type — arrives as a readable diff in the pull request that makes it, rather than buried in a rolled-up declaration bundle. Nothing kept it current. `extract-api` runs api-extractor with `--local`, which rewrites the report rather than failing on drift, and neither half of the safety net named `etc/`: the README's `git add` line listed only the two `dist` paths, and the verifier scoped its `git status` to the same two. So changing a public API and following the README committed a fresh `dist` alongside a stale report, and CI passed — it regenerates `etc/` on every run and then looks away. Add both `etc/` directories to the check and to the README, and rename the constant, the messages, and the section heading to say "build output" now that they cover more than `dist`. The heading rename moves its anchor, so the in-page link to it moves too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012F9WPtSNrcpEAfFdu3trBw * fix: extract each package's API in an order that keeps its rollup `pnpm nx run-many -t extract-api` — what CI ran, what the README told you to run, and what the freshness check's failure message printed — finishes with `packages/utilities/dist/index.d.ts` holding tsc's 13-line re-export stub instead of api-extractor's 648-line rollup. `packages/platform`, `libs/shared`, `libs/shared-react` and `libs/test-data` each carry a TypeScript project reference to `packages/utilities`, so their `build` emits utilities' declarations into utilities' own `outDir`. That write happens below nx's task graph: `build` dependsOn `^build` and `extract-api` dependsOn only its own `build`, so nothing orders `utilities:extract-api` against `platform-editor:build`, and whichever writes last wins. CI has been passing on scheduling luck — the runner happens to overlap the two so the rollup lands second. A pristine clone here loses that race every time, which means following the documented steps committed a broken type surface: that stub re-exports `./converters/**`, the shape that collided xmldom in paranext-core. Extract one package per nx invocation, dependents before the packages they depend on, so the last write is always an `extract-api`. Cache phasing was tried first and does not work — `platform-editor:build` misses the cache and re-runs. `scripts/published-packages.mjs` holds the ordered list and the rationale, and is now the single source for the verified paths too, so the rebuild order and the checked paths cannot drift apart. Issue #5 removes the need for any of this ordering; until then the script is the only supported way to rebuild. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012F9WPtSNrcpEAfFdu3trBw * Removed temporary doc --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Nothing in Platform.Bible consumes @eten-tech-foundation/scribe-editor, and this repo's CLAUDE.md already described the package as not currently maintained and told readers not to use it as a reference. Carrying it means building, typechecking, and testing a package no consumer depends on. Kept as its own commit so the diff against eten-tech-foundation/scripture-editors stays easy to reason about — the rest of the adoption changes touch documentation and workflows, this one touches code. Removes packages/scribe and the wiring it left behind: the tsconfig project reference, the eslint ignore entry, the README demo/package table rows and their npm badges, the mermaid graph nodes, the CLAUDE.md guidance bullet, and the pnpm-lock workspace entry. Also drops scribe's NoteEditor as the worked example in libs/shared's usjBaseNodes comment; the point that comment makes — a non-react host registers the class straight from usjBaseNodes — still stands without it. The history is untouched: scribe's commits and its scribe_v* release tags are still here. Verified with build, typecheck, and test across the remaining nine projects, run in that order (running typecheck before build fails on unmodified main too — TS project references need the declarations built first).

`nx dev scribe` names a package that no longer exists, and the architecture diagram and the marker-menu comment still listed it among the hosts that build on `shared-react`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JsciPEjXb7X4DZzahoWPwo

Both changelog preambles named `Publish Scribe Package` as the only publish workflow; that workflow went with the paranext adoption, and `Publish` now tags releases of both packages without editing either changelog. The utilities entry for `usjToUsxString` also carried a caveat that the Scribe editor did not yet carry the USJ version through, which no longer applies with the package gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The usjBaseNodes comment this branch rewrites is carried into the bundle, so the committed output drifted from the source by that comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

irahopkinson

@irahopkinson+AI made 5 comments.
Reviewable status: 0 of 85 files reviewed, 5 unresolved discussions (waiting on tjcouch-sil).

a discussion (no related file):
Thanks, TJ, for getting back to this cleanup.

I reviewed the removal of the scribe package and found 5 findings, all minor: stale comments and docs wording, and one leftover config entry. Nothing here breaks the build or CI. Each comment shows its severity, and checked and confirmed means the finding was verified against the code at this head.

Findings that could not be attached to a line in this diff

#5 (low · checked and confirmed) pnpm-workspace.yaml:12 - pnpm-workspace.yaml still allows Tailwind's oxide package to run install scripts although no workspace package installs it any more.

What happens: The PR removes the packages/scribe importer from pnpm-lock.yaml, and with it Tailwind's postcss -> node -> oxide package chain; the lockfile no longer names oxide. The remaining tailwindcss dependency of demos/platform/package.json:21 does not pull it in. onlyBuiltDependencies at pnpm-workspace.yaml:12 still lists the Tailwind oxide package.

Why it matters: Maintenance only, with no effect on install or build, since pnpm ignores entries for packages that are not installed; someone auditing which packages may run native install scripts gets a misleading list.

Fix: Delete the Tailwind oxide line (pnpm-workspace.yaml:12) from onlyBuiltDependencies, then run pnpm install and commit pnpm-lock.yaml if its settings block changes.

How this was checked: The base lockfile listed Tailwind's oxide package and its per-platform builds (for example the oxide android-arm64 build, version 4.2.2, at pnpm-lock.yaml:3348 on the merge base). At the head, grep -c oxide pnpm-lock.yaml returns 0. The only remaining tailwind packages are tailwindcss 4.2.2 and tailwindcss-animate (pnpm-lock.yaml:307-312, 7127), which do not pull oxide in. The only tailwind dependency of demos/platform is tailwindcss (demos/platform/package.json:21), and no use of Tailwind's vite or postcss plugin packages exists in demos. pnpm-workspace.yaml:12 still lists the oxide package in onlyBuiltDependencies and is not in this PR's diff. The impact is cosmetic.

Not attached to a line: this finding is in a file the pull request does not change, so the review has no file to attach a comment to

(AI-assisted, with my guidance)

packages/platform/CHANGELOG.md line 8 at r1 (raw file):

Entries under **Unreleased** describe changes that are committed but not yet published. Move them
under a version heading when that version of this package is released. (The `Publish` workflow

#3 - low · checked and confirmed

The changelog intro says Unreleased entries are "not yet published" while pointing readers to a README section that says these packages are not published to npm at all.

What happens: The PR rewrites packages/platform/CHANGELOG.md:8-9 to say a version is "released" and to cite the Publish workflow, which tags the release. Line 7 still says entries are "committed but not yet published". README.md:465 says these packages are not published to npm, and .github/workflows/publish.yml:4-5 describes a release as a repository tag.

Why it matters: Docs only: "published" and "released" mean different things within one paragraph, so a reader may expect an npm version for Unreleased entries that will never exist.

Fix: In packages/platform/CHANGELOG.md:7, change "not yet published" to "not yet released (tagged)" so the paragraph uses one term.

How this was checked: packages/platform/CHANGELOG.md:7 says "committed but not yet published", while lines 8-9 (rewritten by this PR) say "released" and point to the README's "Releasing" section. README.md:465 states "These packages are not published to npm", and .github/workflows/publish.yml:4-5 says a release is a repository tag. Line 7 itself is unchanged by this PR (the diff only touches lines 8-9), so the word "published" predates it, but the PR now leaves the paragraph using both terms. Docs only.

Similar fix as #4.

libs/shared-react/src/plugins/usj/UsjNodesMenuPlugin.tsx line 200 at r1 (raw file):

 * `\`/Enter menu mounted by `UsjNodesMenuPlugin` in place of the legacy typeahead when
 * `editableHarness` is supplied. This is the real marker menu for every host that does not
 * bring its own UI — the repo's demos. A host that does

#1 - low · checked and confirmed

The doc comment names "the repo's demos" as where the editable marker menu appears, but the PERF demos never mount it and the Platform demo shows it only after "Has external UI" is unchecked.

What happens: UsjNodesMenuPlugin is mounted only by packages/platform/src/editor/Editor.tsx:2063-2080, under scrRef && !hasExternalUI. demos/perf-react/src/app/Editor.tsx:253 mounts PerfNodesMenuPlugin instead, and demos/platform/src/app/app.tsx:141 starts with hasExternalUI set to true. The rewritten sentence at UsjNodesMenuPlugin.tsx:199-200 also leaves line 200 short because the paragraph was not re-wrapped.

Why it matters: Maintenance only, with no behavior effect: a reader looking for the menu in the demos looks in the wrong places.

Fix: In this doc comment, replace "the repo's demos" with "packages/platform's Editor when hasExternalUI is false (e.g. demos/platform with 'Has external UI' unchecked)" and re-wrap the paragraph. The sourcemap carries this comment, so run node scripts/rebuild-committed-output.mjs, confirm node scripts/verify-committed-dist.mjs passes, and commit the regenerated packages/platform/dist.

How this was checked: UsjNodesMenuPlugin is mounted in exactly one place outside tests, packages/platform/src/editor/Editor.tsx:2064, under scrRef && !hasExternalUI (line 2063). demos/perf-react/src/app/Editor.tsx:28,253 uses PerfNodesMenuPlugin and never UsjNodesMenuPlugin (no other file under demos references either plugin). demos/platform/src/app/app.tsx:141 initialises hasExternalUI to true (checkbox at :517-521), so the platform demo shows the menu only after the user unchecks "Has external UI". The PR edited this comment (libs/shared-react/src/plugins/usj/UsjNodesMenuPlugin.tsx:199-200, dropping "and the scribe package"), and line 200 is now short. The committed packages/platform/dist/index.js.map embeds this exact text ("bring its own UI - the repo's demos. A host that does"), so the dist regeneration obligation is real. Maintenance only, no behavior effect.

packages/utilities/CHANGELOG.md line 8 at r1 (raw file):

Entries under **Unreleased** describe changes that are committed but not yet published. Move them
under a version heading when that version of this package is released. (The `Publish` workflow

#4 - low · checked and confirmed

The changelog intro says Unreleased entries are "not yet published" while pointing readers to a README section that says these packages are not published to npm at all.

What happens: The PR rewrites packages/utilities/CHANGELOG.md:8-9 to say a version is "released" and to cite the Publish workflow, which tags the release. Line 7 still says entries are "committed but not yet published". README.md:465 says these packages are not published to npm, and .github/workflows/publish.yml:4-5 describes a release as a repository tag.

Why it matters: Docs only: "published" and "released" mean different things within one paragraph, so a reader may expect an npm version for Unreleased entries that will never exist.

Fix: In packages/utilities/CHANGELOG.md:7, change "not yet published" to "not yet released (tagged)" so the paragraph uses one term.

How this was checked: packages/utilities/CHANGELOG.md:7 reads "committed but not yet published", and lines 8-9, rewritten by this PR, say "released" and cite the Publish workflow. README.md:465 says the packages are not published to npm, and publish.yml:4-5 defines a release as a repository tag. Line 7 is untouched by the PR (the diff hunk covers only lines 8-9, plus a separate removal further down), so the wording is pre-existing but now sits beside the new term in the same paragraph. Docs only.

Similar fix as #3.

libs/shared/src/nodes/usj/index.ts line 77 at r1 (raw file):

  // (displayRunSync.utils.ts's shared $syncDisplayRun driver, parameterized by each kind's own
  // descriptor) constructs one whenever it heals a run forward from a loose or missing shape —
  // every USJ-shaped editor needs the class registered, not only shared-react's (a non-react host

#2 - low · checked and confirmed

The usjBaseNodes comment justifies registering AttributeRunNode by citing "a non-react host" that builds its editor straight from usjBaseNodes, but no such production host remains in the repo.

What happens: The parenthetical at libs/shared/src/nodes/usj/index.ts:77-78 drops scribe's NoteEditor as its example but keeps the claim. The remaining callers that build an editor from usjBaseNodes without usjReactNodes are test setup: createBasicTestEnvironment's default at libs/shared/src/nodes/usj/test.utils.ts:50 and helpers such as packages/platform/src/editor/adaptors/twin-pin.test-helpers.tsx:55.

Why it matters: Maintenance only, with no behavior effect: a reader who searches for the production host finds none and may conclude AttributeRunNode can be dropped from the base list.

Fix: Edit the comment at libs/shared/src/nodes/usj/index.ts:77-78 so it names a real caller: replace the parenthetical with "(createBasicTestEnvironment in test.utils.ts builds its editor straight from usjBaseNodes with no react-specific node list)", or end the sentence at "not only shared-react's". Keep tracker IDs and task labels out of the comment. The comment is bundled into packages/platform/dist/index.js, so run node scripts/rebuild-committed-output.mjs, confirm node scripts/verify-committed-dist.mjs passes, and commit the regenerated dist.

How this was checked: A tree-wide grep for usjBaseNodes (excluding packages/platform/dist and node_modules) finds one non-test consumer, libs/shared-react/src/nodes/usj/index.ts:17, which spreads it into the react node list. Every other hit is a test or test helper: libs/shared/src/nodes/usj/test.utils.ts:50 (the createBasicTestEnvironment default), many *.test.ts files, and packages/platform/src/editor/adaptors/twin-pin.test-helpers.tsx:55. demos/platform/src/app/NoteEditor.tsx does not reference usjBaseNodes, and packages/scribe is gone. libs/shared is bundled into the platform package rather than published separately (scripts/published-packages.mjs:7), so no external host is evidenced either. The PR edit at libs/shared/src/nodes/usj/index.ts:77-78 removes the scribe example but keeps the "a non-react host builds its editor straight from usjBaseNodes" claim, which now names no real caller.

…owance Review follow-ups to the scribe removal: - UsjNodesMenuPlugin's doc comment named "the repo's demos" as where the editable marker menu shows; only demos/platform mounts it, and only with "Has external UI" unchecked. - The usjBaseNodes comment kept "a non-react host" as its reason for registering AttributeRunNode after its scribe example went. The remaining such caller is libs/shared's own createBasicTestEnvironment, so the comment names it. - Both changelog preambles said "published" in one sentence and "released" in the next; these packages are released as repository tags and never published to npm. - pnpm-workspace.yaml still let @tailwindcss/oxide run install scripts, but nothing installs it once scribe's Tailwind toolchain is gone. Regenerates platform-editor's committed dist, which bundles the two comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tjcouch-sil · 2026-11-05T16:18:39Z

Thanks, Ira. All five are fixed in dabe969, and I checked each one against the code before changing it.

#1 (UsjNodesMenuPlugin doc comment): you're right, only packages/platform's Editor mounts it, so in this repo it shows up only in demos/platform with "Has external UI" unchecked. The comment now says that, and the paragraph is re-wrapped.

#2 (usjBaseNodes comment): confirmed. usjBaseNodes isn't part of platform-editor's public API, so the only remaining non-react caller is test setup. I took your first suggestion and named createBasicTestEnvironment, which keeps a reader from concluding AttributeRunNode can leave the base list.

#3 / #4 (changelog intros): both now say "not yet released", so each paragraph uses one term.

#5 (@tailwindcss/oxide in onlyBuiltDependencies): removed. pnpm install --frozen-lockfile passes, and pnpm-lock.yaml doesn't change, since pnpm doesn't record onlyBuiltDependencies in the lockfile settings.

The two comments are bundled into packages/platform/dist, so I regenerated it, and verify-committed-dist.mjs passes. lint and typecheck pass on shared and shared-react.

irahopkinson

@irahopkinson+AI made 5 comments and resolved 5 discussions.
Reviewable status: 0 of 86 files reviewed, all discussions resolved.

a discussion (no related file):