diff --git a/backlog/tasks/task-322 - Fix-failing-CI-checks-on-PR-57.md b/backlog/tasks/task-322 - Fix-failing-CI-checks-on-PR-57.md new file mode 100644 index 00000000..6fd6ea1d --- /dev/null +++ b/backlog/tasks/task-322 - Fix-failing-CI-checks-on-PR-57.md @@ -0,0 +1,58 @@ +--- +id: TASK-322 +title: Fix failing CI checks on PR 57 +status: Done +assignee: + - codex +created_date: '2026-05-03 06:27' +updated_date: '2026-05-03 06:31' +labels: + - ci + - bug +dependencies: [] +references: + - 'https://github.com/ksyasuda/SubMiner/pull/57' +priority: high +--- + +## Description + + +Investigate and fix failing GitHub Actions checks on PR #57 (`tokenizer-updates`). Scope: use CI logs to identify root cause, apply focused local fix, and verify with relevant local checks. + + +## Acceptance Criteria + +- [x] #1 Failing GitHub Actions check root cause is identified from logs. +- [x] #2 A focused code/test/docs fix is applied locally. +- [x] #3 Relevant local verification passes or blocked reason is documented. +- [x] #4 PR checks are rechecked or next CI action is documented. +- [x] #5 Actionable CodeRabbit PR comments are inspected and addressed or documented as non-actionable. + + +## Implementation Plan + + +1. Fix CI changelog lint by adding a valid `type` frontmatter value to `changes/319-interjection-annotation-filter.md`. +2. Address unresolved CodeRabbit threads: + - `scripts/test-plugin-session-bindings.lua`: make `.tmp` creation portable across Unix/Windows shells. + - `src/core/services/tokenizer.ts`: pass `TokenizerAnnotationOptions` through `stripSubtitleAnnotationMetadata` paths so `sourceText` is honored. + - `src/main/runtime/mpv-main-event-main-deps.ts`: align overlay-runtime quit-on-disconnect predicate with `hasInitialPlaybackQuitOnDisconnectArg`. + - `src/renderer/handlers/mouse.test.ts`: make `elementFromPoint` stubs coordinate-sensitive. +3. Run focused checks: `bun run changelog:lint`, relevant tokenizer/main/mouse tests, and plugin Lua test path if available. +4. Recheck PR checks/comments after local verification. + + +## Implementation Notes + + +CI root cause: GitHub Actions `build-test-audit` failed during `bun run changelog:lint`; `changes/319-interjection-annotation-filter.md` must declare `type` as one of `added`, `changed`, `fixed`, `docs`, `internal`. Scope expanded by user to also address CodeRabbit comments on PR #57. + +Implemented CI changelog metadata fix and unresolved CodeRabbit feedback locally. Full verification run: `bun run changelog:lint`, focused tests, `bun run typecheck`, `bun run test:fast`, `bun run test:env`, `bun run build`, `bun run test:smoke:dist`, `bun run format:check:src`. Rechecked PR checks: remote `build-test-audit` still shows the old failing run until this branch is pushed; CodeRabbit remains pending remotely until review reruns. + + +## Final Summary + + +Fixed PR #57 CI failure by converting `changes/319-interjection-annotation-filter.md` to valid changelog fragment metadata. Addressed unresolved CodeRabbit feedback by making plugin test `.tmp` creation portable, threading tokenizer annotation options through metadata stripping, aligning quit-on-disconnect predicates for Jellyfin playback, and strengthening mouse hit-test assertions. Also formatted two existing PR files required by the source format gate. Verification passed locally: changelog lint, focused tests, typecheck, test:fast, test:env, build, smoke dist, and format check. Remote PR checks still show the previous failed `build-test-audit` run until these local changes are pushed. + diff --git a/changes/319-interjection-annotation-filter.md b/changes/319-interjection-annotation-filter.md index 78fd13d8..244665e4 100644 --- a/changes/319-interjection-annotation-filter.md +++ b/changes/319-interjection-annotation-filter.md @@ -1 +1,4 @@ -fix: suppress annotations for ハァ-style interjection subtitles +type: fixed +area: tokenizer + +- Tokenizer: Suppress annotations for ハァ-style interjection subtitles. diff --git a/scripts/test-plugin-session-bindings.lua b/scripts/test-plugin-session-bindings.lua index da433a52..c2f8ea5d 100644 --- a/scripts/test-plugin-session-bindings.lua +++ b/scripts/test-plugin-session-bindings.lua @@ -10,7 +10,9 @@ local function assert_true(condition, message) end local artifact_path = ".tmp/test-plugin-session-bindings.json" -os.execute("mkdir -p .tmp") +local is_windows = package.config:sub(1, 1) == "\\" +local mkdir_cmd = is_windows and "mkdir .tmp >NUL 2>NUL" or "mkdir -p .tmp" +os.execute(mkdir_cmd) local handle = assert(io.open(artifact_path, "w")) handle:write("__SESSION_BINDINGS__") handle:close() diff --git a/src/core/services/tokenizer.ts b/src/core/services/tokenizer.ts index 90a06f4c..63be5eba 100644 --- a/src/core/services/tokenizer.ts +++ b/src/core/services/tokenizer.ts @@ -160,7 +160,7 @@ async function applyAnnotationStage( options: TokenizerAnnotationOptions, ): Promise { if (!hasAnyAnnotationEnabled(options)) { - return stripSubtitleAnnotationMetadata(tokens); + return stripSubtitleAnnotationMetadata(tokens, options); } if (!annotationStageModulePromise) { @@ -179,7 +179,10 @@ async function applyAnnotationStage( ); } -async function stripSubtitleAnnotationMetadata(tokens: MergedToken[]): Promise { +async function stripSubtitleAnnotationMetadata( + tokens: MergedToken[], + options: TokenizerAnnotationOptions, +): Promise { if (tokens.length === 0) { return tokens; } @@ -189,7 +192,7 @@ async function stripSubtitleAnnotationMetadata(tokens: MergedToken[]): Promise annotationStage.stripSubtitleAnnotationMetadata(token)); + return tokens.map((token) => annotationStage.stripSubtitleAnnotationMetadata(token, options)); } export function createTokenizerDepsRuntime( @@ -683,25 +686,23 @@ async function parseWithYomitanInternalParser( return null; } const normalizedSelectedTokens = normalizeSelectedYomitanTokens( - selectedTokens.map( - (token): MergedToken => { - const posMetadata = getYomitanWordClassPosMetadata(token.wordClasses); - return { - surface: token.surface, - reading: token.reading, - headword: token.headword, - startPos: token.startPos, - endPos: token.endPos, - partOfSpeech: posMetadata.partOfSpeech, - pos1: posMetadata.pos1, - isMerged: true, - isKnown: false, - isNPlusOneTarget: false, - isNameMatch: token.isNameMatch ?? false, - frequencyRank: token.frequencyRank, - }; - }, - ), + selectedTokens.map((token): MergedToken => { + const posMetadata = getYomitanWordClassPosMetadata(token.wordClasses); + return { + surface: token.surface, + reading: token.reading, + headword: token.headword, + startPos: token.startPos, + endPos: token.endPos, + partOfSpeech: posMetadata.partOfSpeech, + pos1: posMetadata.pos1, + isMerged: true, + isKnown: false, + isNPlusOneTarget: false, + isNameMatch: token.isNameMatch ?? false, + frequencyRank: token.frequencyRank, + }; + }), ); if (deps.getYomitanGroupDebugEnabled?.() === true) { diff --git a/src/core/services/tokenizer/annotation-stage.ts b/src/core/services/tokenizer/annotation-stage.ts index d8193365..543541ba 100644 --- a/src/core/services/tokenizer/annotation-stage.ts +++ b/src/core/services/tokenizer/annotation-stage.ts @@ -507,8 +507,11 @@ export function shouldExcludeTokenFromSubtitleAnnotations(token: MergedToken): b return sharedShouldExcludeTokenFromSubtitleAnnotations(token); } -export function stripSubtitleAnnotationMetadata(token: MergedToken): MergedToken { - return sharedStripSubtitleAnnotationMetadata(token); +export function stripSubtitleAnnotationMetadata( + token: MergedToken, + options: AnnotationStageOptions = {}, +): MergedToken { + return sharedStripSubtitleAnnotationMetadata(token, options); } function computeTokenKnownStatus( diff --git a/src/core/services/tokenizer/parser-enrichment-stage.ts b/src/core/services/tokenizer/parser-enrichment-stage.ts index 86f6af4d..3f32741e 100644 --- a/src/core/services/tokenizer/parser-enrichment-stage.ts +++ b/src/core/services/tokenizer/parser-enrichment-stage.ts @@ -310,8 +310,7 @@ function fillMissingPos1BySurfaceSequence( let cursor = 0; return tokens.map((token) => { - const hasCompletePosMetadata = - token.pos1?.trim() && token.pos2?.trim() && token.pos3?.trim(); + const hasCompletePosMetadata = token.pos1?.trim() && token.pos2?.trim() && token.pos3?.trim(); if (hasCompletePosMetadata) { return token; } diff --git a/src/main/runtime/mpv-main-event-main-deps.ts b/src/main/runtime/mpv-main-event-main-deps.ts index 2f0445b7..44399d58 100644 --- a/src/main/runtime/mpv-main-event-main-deps.ts +++ b/src/main/runtime/mpv-main-event-main-deps.ts @@ -85,12 +85,16 @@ export function createBuildBindMpvMainEventHandlersMainDepsHandler(deps: { hasInitialPlaybackQuitOnDisconnectArg: () => Boolean( deps.appState.initialArgs?.managedPlayback || - deps.appState.initialArgs?.jellyfinPlay || - deps.appState.initialArgs?.youtubePlay, + deps.appState.initialArgs?.jellyfinPlay || + deps.appState.initialArgs?.youtubePlay, ), isOverlayRuntimeInitialized: () => deps.appState.overlayRuntimeInitialized, shouldQuitOnDisconnectWhenOverlayRuntimeInitialized: () => - Boolean(deps.appState.initialArgs?.managedPlayback || deps.appState.initialArgs?.youtubePlay), + Boolean( + deps.appState.initialArgs?.managedPlayback || + deps.appState.initialArgs?.jellyfinPlay || + deps.appState.initialArgs?.youtubePlay, + ), isQuitOnDisconnectArmed: () => deps.getQuitOnDisconnectArmed(), scheduleQuitCheck: (callback: () => void) => deps.scheduleQuitCheck(callback), isMpvConnected: () => Boolean(deps.appState.mpvClient?.connected), diff --git a/src/renderer/handlers/mouse.test.ts b/src/renderer/handlers/mouse.test.ts index 35a2307e..f6e64ff3 100644 --- a/src/renderer/handlers/mouse.test.ts +++ b/src/renderer/handlers/mouse.test.ts @@ -1347,7 +1347,8 @@ test('window resize allows primary hover pause from a real mouseenter over subti configurable: true, value: { addEventListener: () => {}, - elementFromPoint: () => ctx.dom.subtitleContainer, + elementFromPoint: (x: number, y: number) => + x === 120 && y === 240 ? ctx.dom.subtitleContainer : null, querySelectorAll: () => [], body: {}, }, @@ -1496,7 +1497,8 @@ test('pointer tracking enables overlay interaction as soon as the cursor reaches bucket.push(listener); documentListeners.set(type, bucket); }, - elementFromPoint: () => ctx.dom.subtitleContainer, + elementFromPoint: (x: number, y: number) => + x === 120 && y === 240 ? ctx.dom.subtitleContainer : null, querySelectorAll: () => [], body: {}, }, diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index 2721cea7..1cbe5404 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -1069,10 +1069,7 @@ test('subtitle annotation CSS underlines JLPT tokens without changing token colo cssText, `#subtitleRoot .word.word-jlpt-n${level}::selection`, ); - assert.ok( - jlptSelectionLockBlock.length > 0, - `word-jlpt-n${level} selection lock should exist`, - ); + assert.ok(jlptSelectionLockBlock.length > 0, `word-jlpt-n${level} selection lock should exist`); assert.match( jlptSelectionLockBlock, new RegExp(