refactor(transcribe): extract t-mark + draw-cue policy into tested helpers (#327)
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m33s
CI / OCR Service Tests (push) Successful in 24s
CI / Backend Unit Tests (push) Successful in 3m42s
CI / fail2ban Regex (push) Successful in 43s
CI / Semgrep Security Scan (push) Successful in 22s
CI / Compose Bucket Idempotency (push) Successful in 1m7s
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m33s
CI / OCR Service Tests (push) Successful in 24s
CI / Backend Unit Tests (push) Successful in 3m42s
CI / fail2ban Regex (push) Successful in 43s
CI / Semgrep Security Scan (push) Successful in 22s
CI / Compose Bucket Idempotency (push) Successful in 1m7s
Review follow-up (Sara, fast-follow): the t no-active-region guard and the draw-cue arm/disarm rule lived inline in the page with no direct coverage. Extracted to pure resolveTrainingMark() (no-op when no region; recognition enrolled flip) and canArmDraw()/shouldDisarmDraw(), each with unit tests (10 cases total). The page now arms the draw cue only via canArmDraw and disarms via shouldDisarmDraw, and routes t through resolveTrainingMark. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit was merged in pull request #728.
This commit is contained in:
14
frontend/src/lib/document/transcription/drawCue.spec.ts
Normal file
14
frontend/src/lib/document/transcription/drawCue.spec.ts
Normal file
@@ -0,0 +1,14 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { canArmDraw, shouldDisarmDraw } from './drawCue';
|
||||||
|
|
||||||
|
describe('draw cue policy', () => {
|
||||||
|
it('arms only in edit mode', () => {
|
||||||
|
expect(canArmDraw('edit')).toBe(true);
|
||||||
|
expect(canArmDraw('read')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('disarms in every mode except edit', () => {
|
||||||
|
expect(shouldDisarmDraw('read')).toBe(true);
|
||||||
|
expect(shouldDisarmDraw('edit')).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
20
frontend/src/lib/document/transcription/drawCue.ts
Normal file
20
frontend/src/lib/document/transcription/drawCue.ts
Normal file
@@ -0,0 +1,20 @@
|
|||||||
|
/**
|
||||||
|
* Policy for the "draw a new region" keyboard cue (the `n` shortcut) in the
|
||||||
|
* transcribe panel — issue #327.
|
||||||
|
*
|
||||||
|
* The cue is only valid while editing: `n` arms it in edit mode, and it must
|
||||||
|
* clear when a region is drawn or when the panel leaves edit mode. Pure so both
|
||||||
|
* rules are testable without mounting the page.
|
||||||
|
*/
|
||||||
|
|
||||||
|
type PanelMode = 'read' | 'edit';
|
||||||
|
|
||||||
|
/** The draw cue may only be armed while in edit mode. */
|
||||||
|
export function canArmDraw(panelMode: PanelMode): boolean {
|
||||||
|
return panelMode === 'edit';
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Leaving edit mode must disarm the draw cue. */
|
||||||
|
export function shouldDisarmDraw(panelMode: PanelMode): boolean {
|
||||||
|
return !canArmDraw(panelMode);
|
||||||
|
}
|
||||||
30
frontend/src/lib/document/transcription/trainingMark.spec.ts
Normal file
30
frontend/src/lib/document/transcription/trainingMark.spec.ts
Normal file
@@ -0,0 +1,30 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { resolveTrainingMark, RECOGNITION_TRAINING_LABEL } from './trainingMark';
|
||||||
|
|
||||||
|
describe('resolveTrainingMark', () => {
|
||||||
|
it('is a no-op (null) when no region is active', () => {
|
||||||
|
expect(resolveTrainingMark(null, [])).toBe(null);
|
||||||
|
expect(resolveTrainingMark(null, [RECOGNITION_TRAINING_LABEL])).toBe(null);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('enrols recognition training when a region is active and not yet enrolled', () => {
|
||||||
|
expect(resolveTrainingMark('ann-1', [])).toEqual({
|
||||||
|
label: RECOGNITION_TRAINING_LABEL,
|
||||||
|
enrolled: true
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('un-enrols when recognition training is already enrolled', () => {
|
||||||
|
expect(resolveTrainingMark('ann-1', [RECOGNITION_TRAINING_LABEL])).toEqual({
|
||||||
|
label: RECOGNITION_TRAINING_LABEL,
|
||||||
|
enrolled: false
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores unrelated document training labels', () => {
|
||||||
|
expect(resolveTrainingMark('ann-1', ['KURRENT_SEGMENTATION'])).toEqual({
|
||||||
|
label: RECOGNITION_TRAINING_LABEL,
|
||||||
|
enrolled: true
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
31
frontend/src/lib/document/transcription/trainingMark.ts
Normal file
31
frontend/src/lib/document/transcription/trainingMark.ts
Normal file
@@ -0,0 +1,31 @@
|
|||||||
|
/**
|
||||||
|
* "Mark for training" (the `t` shortcut) decision logic — issue #327.
|
||||||
|
*
|
||||||
|
* Training enrollment is document-level — two fixed script-type chips
|
||||||
|
* (KURRENT_RECOGNITION / KURRENT_SEGMENTATION); there is no per-region training
|
||||||
|
* flag yet (that arrives with #321). `t` toggles the primary recognition
|
||||||
|
* enrollment and is a silent no-op unless a region is active, so it reads as an
|
||||||
|
* action on the region the transcriber is working on.
|
||||||
|
*
|
||||||
|
* Pure so the no-op-when-no-region guard and the enrolled flip are testable
|
||||||
|
* without mounting the page.
|
||||||
|
*/
|
||||||
|
export const RECOGNITION_TRAINING_LABEL = 'KURRENT_RECOGNITION';
|
||||||
|
|
||||||
|
export type TrainingMarkToggle = { label: string; enrolled: boolean };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide the recognition-training toggle for the active region, or null when no
|
||||||
|
* region is active (the `t` shortcut is then a silent no-op).
|
||||||
|
*
|
||||||
|
* @param activeAnnotationId the currently active region, or null
|
||||||
|
* @param currentLabels the document's currently enrolled training labels
|
||||||
|
*/
|
||||||
|
export function resolveTrainingMark(
|
||||||
|
activeAnnotationId: string | null,
|
||||||
|
currentLabels: readonly string[]
|
||||||
|
): TrainingMarkToggle | null {
|
||||||
|
if (!activeAnnotationId) return null;
|
||||||
|
const enrolled = !currentLabels.includes(RECOGNITION_TRAINING_LABEL);
|
||||||
|
return { label: RECOGNITION_TRAINING_LABEL, enrolled };
|
||||||
|
}
|
||||||
@@ -13,6 +13,8 @@ import { transcribeShortcuts } from '$lib/shared/actions/transcribeShortcuts';
|
|||||||
import { createOcrJob } from '$lib/ocr/useOcrJob.svelte';
|
import { createOcrJob } from '$lib/ocr/useOcrJob.svelte';
|
||||||
import { createTranscriptionBlocks } from '$lib/document/transcription/useTranscriptionBlocks.svelte';
|
import { createTranscriptionBlocks } from '$lib/document/transcription/useTranscriptionBlocks.svelte';
|
||||||
import { stepRegion } from '$lib/document/transcription/regionNavigation';
|
import { stepRegion } from '$lib/document/transcription/regionNavigation';
|
||||||
|
import { resolveTrainingMark } from '$lib/document/transcription/trainingMark';
|
||||||
|
import { canArmDraw, shouldDisarmDraw } from '$lib/document/transcription/drawCue';
|
||||||
import { createFileLoader } from '$lib/document/viewer/useFileLoader.svelte';
|
import { createFileLoader } from '$lib/document/viewer/useFileLoader.svelte';
|
||||||
import { scrollToCommentFromQuery } from '$lib/shared/utils/deepLinkScroll';
|
import { scrollToCommentFromQuery } from '$lib/shared/utils/deepLinkScroll';
|
||||||
import { getConfirmService } from '$lib/shared/services/confirm.svelte';
|
import { getConfirmService } from '$lib/shared/services/confirm.svelte';
|
||||||
@@ -112,17 +114,9 @@ function toggleMode() {
|
|||||||
if (canWrite) panelMode = panelMode === 'read' ? 'edit' : 'read';
|
if (canWrite) panelMode = panelMode === 'read' ? 'edit' : 'read';
|
||||||
}
|
}
|
||||||
|
|
||||||
// Training enrollment is document-level — two fixed script-type chips
|
|
||||||
// (KURRENT_RECOGNITION / KURRENT_SEGMENTATION); there is no per-region training
|
|
||||||
// flag (that would arrive with #321). "t" toggles the primary recognition
|
|
||||||
// enrollment and stays a no-op unless a region is active, so it reads as an
|
|
||||||
// action on the region the transcriber is working on.
|
|
||||||
const RECOGNITION_TRAINING_LABEL = 'KURRENT_RECOGNITION';
|
|
||||||
|
|
||||||
function toggleTrainingMark() {
|
function toggleTrainingMark() {
|
||||||
if (!activeAnnotationId) return;
|
const toggle = resolveTrainingMark(activeAnnotationId, doc.trainingLabels ?? []);
|
||||||
const enrolled = !(doc.trainingLabels ?? []).includes(RECOGNITION_TRAINING_LABEL);
|
if (toggle) transcription.toggleTrainingLabel(toggle.label, toggle.enrolled);
|
||||||
transcription.toggleTrainingLabel(RECOGNITION_TRAINING_LABEL, enrolled);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function deleteCurrentRegion() {
|
function deleteCurrentRegion() {
|
||||||
@@ -131,7 +125,7 @@ function deleteCurrentRegion() {
|
|||||||
|
|
||||||
// Disarm the draw cue whenever we leave edit mode.
|
// Disarm the draw cue whenever we leave edit mode.
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
if (panelMode !== 'edit') drawArmed = false;
|
if (shouldDisarmDraw(panelMode)) drawArmed = false;
|
||||||
});
|
});
|
||||||
|
|
||||||
const shortcutOptions = {
|
const shortcutOptions = {
|
||||||
@@ -142,7 +136,9 @@ const shortcutOptions = {
|
|||||||
goToPrevRegion: () => goToRegion(-1),
|
goToPrevRegion: () => goToRegion(-1),
|
||||||
toggleMode,
|
toggleMode,
|
||||||
closePanel: () => (transcribeMode = false),
|
closePanel: () => (transcribeMode = false),
|
||||||
startDrawMode: () => (drawArmed = true),
|
startDrawMode: () => {
|
||||||
|
if (canArmDraw(panelMode)) drawArmed = true;
|
||||||
|
},
|
||||||
toggleTrainingMark,
|
toggleTrainingMark,
|
||||||
deleteCurrentRegion,
|
deleteCurrentRegion,
|
||||||
openCheatsheet: () => (cheatsheetOpen = true)
|
openCheatsheet: () => (cheatsheetOpen = true)
|
||||||
|
|||||||
Reference in New Issue
Block a user