Skip to content

Add onAnalysisChanged for Scripture edits - #468

Merged
imnasnainaec merged 3 commits into
mainfrom
feat/438-on-analysis-changed
Oct 19, 2026
Merged

imnasnainaec merged 3 commits into
mainfrom
feat/438-on-analysis-changed

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Closes #438. Part of Phase 2 (#425).

Summary

A Scripture edit to the open book refreshes the view again.

BookService watches each project's open book: the one whose outline was last asked for. The watch is separate from the book cache, because the whole-text read pushes the open book out of the cache. On an edit, the host emits interlinearizer.onAnalysisChanged. The WebView then reloads the book.

getBookOutline(project, book)
  watchOpenBook(project, book)        # replaces the project's previous open-book watch

USJ edit to the open book
  release the held book, if it is that book
  onOpenBookTextChanged listeners
    emit onAnalysisChanged { sourceProjectId, changeId, scriptureEdit: { bookId } }

open-book watch fails to read the book
  drop it, if still current       # the next outline watches again

WebView: useInterlinearizerBookData
  on onAnalysisChanged naming its project and book
    reload()  ->  getTokenizedBook  ->  outline and pages refetch

closeProject and closeAllProjects also stop the open-book watches.

Evidence

  • Before: on main, editing a verse in the Scripture editor leaves the open Interlinearizer view on the old text. The new text shows only once the book is reopened.
    After: new tests in bookService.test.ts, main.test.ts and useInterlinearizerBookData.test.ts pass, among them:
    outline GEN; read EXO and LEV whole; edit GEN
      -> listener called with (src, GEN)
      -> getTokenizedBook(src, GEN) reads the edited text
    outline GEN; open watch fails to read it; outline GEN again; edit GEN
      -> listener called with (src, GEN)
    event { other project | other book } -> no refetch
    
    The full suite passes at 100% coverage.

Manual test plan

  1. Open a project in the Interlinearizer at Genesis. Open the same project in the Scripture editor.
  2. Change a word in a verse on screen. The Interlinearizer shows the new word without reopening.
  3. Open the Concordance or the Analysis Catalog, so the whole text gets read. Edit again. The view still refreshes.
  4. Scroll to a later chapter and edit a verse there. The view keeps its place and shows the edit.
  5. Switch the Interlinearizer to Exodus, then edit Genesis. Nothing reloads. Edit Exodus. The view refreshes.
  6. Close the tab and reopen it. Edits still refresh the view.

Deviations from the plan

  • The event names one whole book. It has no wider scope (the plan's "a whole book or scope").
    • Its only producer edits one book.
    • Wider scopes, and the fields 3k adds, are better shaped once they have producers.
  • Done-when gap: a book with no text, or one that fails to tokenize, isn't watched. An edit that fixes it doesn't refresh the view.

Comments to post on other issues

#439 (2e): posted

#467 (2c): posted

#449 (3j): posted

#450 (3k): posted

#459 (5d): posted

Decisions with a close contender

  • A separate open-book watch over keeping the open book in the cache next to the latest book.
    • The contender is closer to the plan's wording: "subscribes to USJ_Book for open books … caches the tokenized book".
    • It would use one subscription per book, and stop the whole-text read from evicting the open book.
    • But it reworks the cache 2e is about to rework for its own read, and keeps another tokenized book per project, which 2e's heap checkpoint should weigh. It's handed to 2e instead.
  • The last outline marks the open book over an explicit watchBook command.
    • The contender closes the done-when gap above.
    • But it adds API surface the plan doesn't have. Its lifecycle would also duplicate the tab-close tracking.
  • Dropping a failed open watch over retrying it at once.
    • The next outline (a book switch, a boundary edit or a reload) watches again.
    • Retrying at once could loop on a book that keeps failing.
  • scriptureEdit: { bookId } over scriptureEdit: { bookIds: string[] }.
    • The array would cover a multi-book scope later.
    • Today it would always hold one book.
  • A random-UUID changeId over a per-host counter.
    • A counter would let a view drop out-of-order events, but it resets when the host restarts.
    • The UUID matches how the repo makes every other id.

Merge Danger

Door: two-way

The event is new, and only this extension listens to it. Reverting removes it cleanly. No stored data changes.

Blast Radius: small

  • The worst case is an extra reload of the open book, or a missed one. A missed reload means an edit that doesn't refresh, as on main today.
  • While the open book is also the held one, it has a second USJ subscription.
  • deactivate waits for the open watch's first delivery before unsubscribing, up to the request timeout (30s by default). It's the same stop() wait Fetch and tokenize books in a host BookService (#435) #460 has for held books.
  • Each refresh shows the Loading… panel briefly and remounts the view, which drops focus and any scroll away from the current reference. Keep the view on screen while a Scripture edit refetches the book #473 tracks keeping the view on screen.

🤖 Generated with Claude Code


This change is Reviewable

Summary by CodeRabbit

  • New Features
    • Scripture text edits now trigger analysis-change notifications that identify the affected project and book.
    • Views showing the edited book refresh automatically, including after navigating to a newly selected book.
  • Bug Fixes
    • Changes to other books or projects no longer cause unnecessary reloads, helping keep displayed content current without refreshing unrelated views. Notifications are limited to edits affecting the last-outlined open book.

BookService watches each source project's open book, the one whose
outline was last asked for, apart from the held-book cache, since the
whole-text read replaces the held book. An edit to it releases the
held text and fires interlinearizer.onAnalysisChanged with the source
project, a change id and the edited book; the WebView reloads the book
when the event names its project and book.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@imnasnainaec imnasnainaec self-assigned this Oct 19, 2026
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bca3a7cd-41d4-4fc2-b6e1-006cea615e8c
📥 Commits

Reviewing files that changed from the base of the PR and between 65f5ee6 and 64c8462.

📒 Files selected for processing: 4
  • src/__tests__/services/bookService.test.ts
  • src/components/InterlinearizerLoader.tsx
  • src/services/bookService.ts
  • src/types/interlinearizer.d.ts
🚧 Files skipped from review as they are similar to previous changes: 1
  • src/services/bookService.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Scripture text changes to an outlined book now produce an interlinearizer.onAnalysisChanged event. The book-data hook reloads when the event matches its requested project and book.

Changes: Scripture edit notifications

Layer / File(s) Summary
Open-book text change tracking
src/services/bookService.ts, src/__tests__/services/bookService.test.ts
The service watches the outlined book for each project and notifies listeners of text changes. It stops watches when a different book is outlined or a project closes. Tests cover watch replacement, failures, and listener cleanup.
Analysis-change event emission
src/types/interlinearizer.d.ts, src/main.ts, src/__tests__/main.test.ts, AGENTS.md
The event includes the source project ID, a change ID, and the edited book ID. Activation emits it for book text changes and registers cleanup for the emitter and listener.
Matching view reload
src/hooks/useInterlinearizerBookData.ts, src/__tests__/hooks/useInterlinearizerBookData.test.ts
The hook reloads when an event matches its project and book IDs. Tests cover matching and non-matching edits, including edits to a newly selected book.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant BookService
  participant Activation
  participant EventEmitter
  participant BookDataHook
  BookService->>Activation: Notify of project and book text change
  Activation->>EventEmitter: Emit interlinearizer.onAnalysisChanged
  EventEmitter->>BookDataHook: Deliver analysis-change event
  BookDataHook->>BookDataHook: Match project and book IDs, then reload
Loading

Suggested reviewers: alex-rawlings-yyc

Merge Risk: ⚪ Minimal · up to 64c84

Edits that repair a book with missing text or failed tokenization can still refresh the open view. No identified issue blocks merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Out of Scope Changes check Warning Most changes support #438, including the watch, event, WebView refetch, tests, and related documentation. The supplied change summary also identifies an unrelated comment-only change in `src/component… Remove the unrelated sidePanel comment change, or provide evidence that it is required for the #438 implementation.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding onAnalysisChanged for Scripture edits.
Linked Issues check Passed Issue #438 requires a source-project onAnalysisChanged event, a Scripture edit that identifies the book, and a WebView refetch. The supplied PR evidence shows that BookService watches the last-out…
Docstring Coverage Passed Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files.
Full details: Linked Issues check

Explanation

Issue #438 requires a source-project onAnalysisChanged event, a Scripture edit that identifies the book, and a WebView refetch. The supplied PR evidence shows that BookService watches the last-outlined book, main.ts emits the event with sourceProjectId, changeId, and scriptureEdit.bookId, and useInterlinearizerBookData refetches only for the matching project and book. The supplied test summary covers emission, filtering, refetch, watch replacement, failure, and cleanup. The broader payload fields belong to later work and are not required for this first producer.

Full details: Out of Scope Changes check

Explanation

Most changes support #438, including the watch, event, WebView refetch, tests, and related documentation. The supplied change summary also identifies an unrelated comment-only change in src/components/InterlinearizerLoader.tsx about sidePanel tab scope. No connection exists between that comment and Scripture-edit refresh behavior.

Resolution

Remove the unrelated sidePanel comment change, or provide evidence that it is required for the #438 implementation.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

A failed open-book watch stayed current, so later outlines of the same
book kept it and edits to that book went unreported. Drop it, if it is
still the current watch, and stop it, since an error on the first
delivery leaves the subscription open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@imnasnainaec
imnasnainaec marked this pull request as ready for review October 19, 2026 18:36
@imnasnainaec imnasnainaec changed the title Add onAnalysisChanged for Scripture edits (#438) Add onAnalysisChanged for Scripture edits Oct 19, 2026

@alex-rawlings-yyc alex-rawlings-yyc left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments were generated by Claude (Claude Code, Opus 5.5) and reviewed by me before posting.

Comment thread src/__tests__/services/bookService.test.ts Outdated
Comment thread src/__tests__/services/bookService.test.ts
Comment thread src/__tests__/services/bookService.test.ts
Comment thread src/hooks/useInterlinearizerBookData.ts
Comment thread src/services/bookService.ts
Comment thread src/services/bookService.ts Outdated
Comment thread src/types/interlinearizer.d.ts Outdated
Give stopping a failed watch its own test, make the replaced-watch test
fail without its guard, and have the close test check the open watch
is unsubscribed. Document watchOpenBook, scope the onAnalysisChanged
doc to the open book, and drop the side panel doc's claim that one
project is routinely open in two tabs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@imnasnainaec imnasnainaec left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imnasnainaec made 7 comments.
Reviewable status: 0 of 9 files reviewed, 7 unresolved discussions (waiting on alex-rawlings-yyc and imnasnainaec).

Comment thread src/__tests__/services/bookService.test.ts Outdated
Comment thread src/__tests__/services/bookService.test.ts
Comment thread src/__tests__/services/bookService.test.ts
Comment thread src/hooks/useInterlinearizerBookData.ts
Comment thread src/services/bookService.ts
Comment thread src/services/bookService.ts Outdated
Comment thread src/types/interlinearizer.d.ts Outdated
@imnasnainaec
imnasnainaec enabled auto-merge (squash) October 19, 2026 21:37

@alex-rawlings-yyc alex-rawlings-yyc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-rawlings-yyc reviewed 9 files and all commit messages, and resolved 7 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

@imnasnainaec
imnasnainaec merged commit 91f9235 into main Oct 19, 2026
10 checks passed
@imnasnainaec
imnasnainaec deleted the feat/438-on-analysis-changed branch October 19, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2d] Add onAnalysisChanged for Scripture edits

2 participants