Model idea: Split linking out from analyses#73
Open
imnasnainaec wants to merge 59 commits into
Open
Conversation
- Add `src/projectStorage.ts` with `createProject`, `getProject`, `listProjects`, and `deleteProject` backed by `papi.storage` - Register `interlinearizer.createProject` command in `main.ts` that prompts for source/target projects, writes the record, and surfaces storage errors as notifications - Add `InterlinearProject` type and `interlinearizer.createProject` signature to shared type declarations - Extend PAPI backend mock with `papi.storage` and `papi.notifications` - Broaden Jest coverage to all `src/**` files - Add full test suites for the storage module and the new command
…ect and add associated test, fix schema inconsistency
…er index/data write to `papi` storage to ensure data doesn't become inaccessible
…update mocks to match real API signatures - If the user picks the same project for both source and target roles, a warning notification is shown and the target picker re-opens until distinct projects are chosen or cancelled. - Includes new tests for this flow, updated platform-bible-react/utils mocks to match current API shapes, a new localized error string, and minor test/parser cleanups.
Introduces three modal components (CreateProjectModal, ProjectMetadataModal, SelectInterlinearProjectModal) with supporting menu contributions, localized strings, command registrations in main.ts, storage helpers in projectStorage.ts, and updated type definitions. Adds full test coverage for all new components and expands existing test suites to cover the new flows.
…nput - `CreateProjectModal`: guard `onClose()` behind a `!newId` early-return so the modal doesn't close when project creation returns no ID - `ProjectMetadataModal`: trim whitespace from the language field before saving and passing to callbacks - Replace `makeHandleProjectCreated` factory with a plain `handleProjectCreated` callback; removes the closure-over-srcId pattern and reads `projectId` from the enclosing scope directly
…r delete/update - `interlinearizer.createProject` now returns the full persisted project as a JSON string instead of just the UUID, so the WebView can populate `activeProject` with authoritative server data rather than reconstructing it locally - `CreateProjectModal.onProjectCreated` callback now receives the parsed `InterlinearProjectSummary` object instead of `(id, writingSystem)` pair - Add `isInterlinearProjectSummary` type guard to `SelectInterlinearProjectModal` and reuse it in the project list filter and the new create flow - Wrap `deleteProject` and `updateProjectMetadata` backend handlers in try/catch with logging and error notifications (previously unhandled rejections) - Register a no-op `interlinearizer.viewProjectInfo` backend command so the platform menu system can surface it; all logic runs in the WebView - Update tests and type declarations to match
…oss/senseRef constraint - Replace `<div>` containers with `<dialog open aria-labelledby="…">` in `CreateProjectModal`, `ProjectMetadataModal`, and `SelectInterlinearProjectModal` for proper accessibility semantics. - In `ProjectMetadataModal.handleSave`, return early when `updateProjectMetadata` resolves with a falsy value (mirrors the existing guard in `CreateProjectModal.handleCreate`). - In `interlinearizer.d.ts`, replace the discriminated union on `Token` and `Phrase` that forbade setting both `gloss`/`glossSenseRef` (or `gloss`/`senseRef`) with optional fields, reflecting the updated rule that `gloss` serves as a local override when both are present. - Add a test for the falsy-return early-exit path in `ProjectMetadataModal`, and add the missing `waitFor` before negative assertions in the `CreateProjectModal` falsy-return test.
…ation - Default analysis language to "und" (undetermined) instead of "en" - Normalize whitespace-only language input to "und" on submit - Disable the create button while submission is in progress - Rethrow storage errors in getProjectsForSource so callers can distinguish an outage from an empty list
…g onProjectCreated Previously, if the backend returned a JSON object that failed isInterlinearProjectSummary, the modal would call onClose() without invoking onProjectCreated, silently dropping the contract. Now it throws, routing through the existing error handler. Also fixes an incomplete fixture in the success test that was missing required summary fields.
…t was still created", update docs
Co-authored-by: alex-rawlings-yyc <alex.rawlings@wycliffe.ca>
…ations on error, disable metadata modal buttons when submitting
…zero-check in ContinuousView explicit
…xt, add ActiveProject - Removes the unused bilingual container types (`InterlinearAlignment`, `InterlinearText`) whose `target` side had no role in the current single-source annotation workflow. - Introduces `ActiveProject` as the runtime pairing of a persisted `InterlinearProject` envelope with its reconstructed `Book[]` source text. - Adds `saveProjectAnalysis` to `projectStorage` as the designated save path for analysis/link mutations.
…0% coverage - InterlinearProject: replace analysisWritingSystem: string with analysisLanguages: string[] (LCM/PT9 carry multiple analysis languages per project; a single string was lossy) and add optional targetProjectId so BT Extension's two-sided Translation can be represented without dropping the target text layer - ActiveProject: add target?: Book[] alongside source so AlignmentLink.targetEndpoints resolve to real tokens at runtime - TokenAnalysis / Phrase: remove gloss ↔ senseRef discriminated union; both fields are now independently optional and may coexist — local gloss takes precedence over the lexicon-derived value when both are present - TokenAnalysis: add excluded?: boolean to carry PT9's Cluster.Excluded flag projectStorage: update createProject / updateProjectMetadata signatures and logic to match; updateProjectMetadata clears targetProjectId when called without it (mirrors name/description clear semantics) - papi-backend mock: add papi.storage (readUserData / writeUserData / deleteUserData) which was missing entirely - Remove two dead branches: parseStrictNumber's n >= 0 guard (the ^\d+$ regex makes negative results impossible) and stableStringify's undefined guard (never reachable via the typed call site) - Add tests for duplicate verse SID throw and new storage behaviour; all 268 tests pass at 100% branch coverage
Cluster.Excluded is a deprecated PT9 tag; carrying it into the model adds noise with no consumer value.
…ag from PT9 XML docs
…s, improve error handling
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
alex-rawlings-yyc
approved these changes
May 14, 2026
Contributor
alex-rawlings-yyc
left a comment
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
I think this all works. I have no problems with you merging this in. I think the order of everything in this file could do with a cleanup, but I'd rather that be done once this is merged.
@alex-rawlings-yyc reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Idea atop #63
I think we may need something like this to reuse the same analysis on multiple (e.g.) phrases with the same surface form. The model adjustments were done manually by me, and all the doc updates were generated by Copilot to match.
This change is
Also, Copilot-generated summary of the change:
Model Change Summary
This change restructures the Interlinearizer data model to separate analysis payloads from their text attachments.
TextAnalysis:segmentAnalysisLinkstokenAnalysisLinksphraseAnalysisLinksstatus, optionalconfidence) now lives on links, not on analysis payload records.SegmentAnalysis,TokenAnalysis,PhraseAnalysis) are now content-focused (surface/linguistic content + provenance) and no longer carry attachment fields.What Changed Structurally
AnalysisLinkplus concrete link types:SegmentAnalysisLink(analysis id ↔Segment.id)TokenAnalysisLink(analysis id ↔ token snapshot)PhraseAnalysisLink(analysis id ↔ ordered token snapshots)SegmentAnalysis.segmentIdremovedTokenAnalysis.tokenremovedPhraseAnalysis.tokensremovedTextAnalysisnow explicitly models two layers:segmentAnalyses,tokenAnalyses,phraseAnalyses)segmentAnalysisLinks,tokenAnalysisLinks,phraseAnalysisLinks)Why
Drift/Staleness Clarification
Impact
segmentId, token snapshot, phrase token list, and assignment status).