Skip to content

Add token commands with parity tests - #488

Open
alex-rawlings-yyc wants to merge 2 commits into
mainfrom
443-token-commands
Open

alex-rawlings-yyc wants to merge 2 commits into
mainfrom
443-token-commands

Conversation

@alex-rawlings-yyc

@alex-rawlings-yyc alex-rawlings-yyc commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Part of #426; closes #443. Stacked on #487.

Summary

The token commands, with the approval rule and parity tests, and the pattern 3e to 3i follow.

interlinearizer.writeGloss(sessionId, tokenRef, surfaceText, value) -> ChangeSet applied
interlinearizer.approveAnalysisForToken(sessionId, tokenRef, surfaceText, analysisId) -> ChangeSet applied

src/services/commands/
  changeBuffer.ts    # staged puts and deletes; reads answer as if applied
  approvalRule.ts    # refuses a second approved link at a token or segment
  tokenCommands.ts   # writeGloss, approveAnalysisForToken
src/__tests__/services/commands/parity.ts   # runParity: one input through reducer and command
  • Commands copy their reducers step for step, against the repository's lookups: per-token forks, find-or-create, folding into an identical payload, held links, approval ties, and removing links whose payload is gone. That is how this reads "payload dedupe and merge"; the catalog's mergeAnalysesInto stays with 3i.
  • AnalysisSessions.edit applies what a command staged as one undo step, after the approval rule passes, and marks the draft dirty. When the draft lists no analysis languages, that step adds the session's. A command staging nothing applies nothing and records no step.
  • getScope joins the seam, so that an edit can read the draft's analysis languages.

Evidence

  • 26 parity cases run one input through the reducer and the command, then compare the analyses, reloaded from storage, in id order. Each also checks that the repository's indexes still match its records.
  • Eleven mutations of the commands each fail a parity case.
  • Unit suite at 100% coverage; lint and tsc clean.

Decisions with a strong alternative

  • A buffer the command stages into, not a diff of a reducer run. Why: the plan rejects running reducers in the host and deriving change sets by diffing.
  • The approval rule refuses only a new breach. A token or segment already holding two approved links stays as it is, as the reducers leave it.
  • Shared content helpers. isEmptyTokenAnalysis and clearAnalysisGloss moved from the reducer into a util both use, with no change in behavior, so the two can't drift.

Deferred

Pool buckets in replies (3k), and the collapse survivor the reducer reports, which only catalog edits read (3i).

Merge Danger

Door: two-way

Nothing calls the new commands. The reducers changed only by importing two helpers.

Blast Radius: the gloss reducers' content checks, through the moved helpers

🤖 Generated with Claude Code


This change is Reviewable

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2941c61a-28aa-4231-891a-e1d2139b603d
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@alex-rawlings-yyc
alex-rawlings-yyc added this pull request to stack #489 October 22, 2026 17:26
@alex-rawlings-yyc alex-rawlings-yyc self-assigned this Oct 22, 2026
@imnasnainaec

Copy link
Copy Markdown
Contributor

A host edit to a draft with no analysis languages writes the gloss under the session's fallback language (en), but analysisLanguages stays []. When the WebView loads, it picks its own language (useDraftProject.ts:389, which gave und here), so the host's glosses don't show.

The WebView's first save stores the language it picked. Edits should do the same: at analysisSessions.ts:230, also set analysisLanguages: [session.analysisLanguage] when the list is empty, even if dirty is already true. No parity test catches this, since the language is set in a hook, not a reducer.

Comment thread src/services/commands/tokenCommands.ts Outdated
}
} else {
target.gloss = { ...target.gloss, [lang]: value };
delete target.glossSenseRef;

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.

Typing a free-form gloss drops glossSenseRef, so editing a lexicon-backed gloss unlinks it. That goes against the model: TokenAnalysis.glossSenseRef says that when gloss and the ref are both present, "glossSenseRef is retained so the lexicon link is not lost." It's also inconsistent with the other two glosses, since morpheme and phrase gloss writes both keep their senseRef.

This is parity with writeGlossOntoAnalysis, so it isn't a bug in this PR. But the host is about to make this behavior permanent, and #227 starts filling these fields from FW Lite. I suggest keeping the ref in both the reducer and the command (drop the delete in each), plus a parity case with a linked analysis. If that's out of scope here, a follow-up issue before 4c (#455) works too.

Clearing is different and should keep dropping the ref: clearAnalysisGloss removes it once no gloss remains. Keeping it there would make an emptied field show the sense gloss again.

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.

Done: the reducer and the command both keep the ref on a rewrite, and the linked-analysis parity case now expects it kept. The catalog's writeGlossOntoAnalysis dropped it too, so it keeps it now as well.

@alex-rawlings-yyc
alex-rawlings-yyc force-pushed the 443-token-commands branch 2 times, most recently from 9d5b5ef to d4487d9 Compare October 23, 2026 21:08
Base automatically changed from 442-derived-indexes to main October 23, 2026 21:26
@alex-rawlings-yyc

Copy link
Copy Markdown
Contributor Author

re: "A host edit to a draft..."

Fixed. When the draft lists no analysis languages, an edit now stores the session's analysis language in the same undo step.

@alex-rawlings-yyc
alex-rawlings-yyc marked this pull request as ready for review October 23, 2026 21:53
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.

[3d] Add token commands with parity tests

2 participants