mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-05-04 00:41:33 -07:00
fix: CI changelog, annotation options threading, and Jellyfin quit
- Add `type: fixed` / `area:` frontmatter to `changes/319` to pass `changelog:lint` - Thread `TokenizerAnnotationOptions` through `stripSubtitleAnnotationMetadata` so `sourceText` is honored - Include `jellyfinPlay` in `shouldQuitOnDisconnectWhenOverlayRuntimeInitialized` predicate - Make mouse test `elementFromPoint` stubs coordinate-sensitive - Make Lua test `.tmp` mkdir portable on Windows
This commit is contained in:
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [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.
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Implementation Plan
|
||||||
|
|
||||||
|
<!-- SECTION:PLAN:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:PLAN:END -->
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
<!-- SECTION:NOTES:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:NOTES:END -->
|
||||||
|
|
||||||
|
## Final Summary
|
||||||
|
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||||
@@ -1 +1,4 @@
|
|||||||
fix: suppress annotations for ハァ-style interjection subtitles
|
type: fixed
|
||||||
|
area: tokenizer
|
||||||
|
|
||||||
|
- Tokenizer: Suppress annotations for ハァ-style interjection subtitles.
|
||||||
|
|||||||
@@ -10,7 +10,9 @@ local function assert_true(condition, message)
|
|||||||
end
|
end
|
||||||
|
|
||||||
local artifact_path = ".tmp/test-plugin-session-bindings.json"
|
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"))
|
local handle = assert(io.open(artifact_path, "w"))
|
||||||
handle:write("__SESSION_BINDINGS__")
|
handle:write("__SESSION_BINDINGS__")
|
||||||
handle:close()
|
handle:close()
|
||||||
|
|||||||
@@ -160,7 +160,7 @@ async function applyAnnotationStage(
|
|||||||
options: TokenizerAnnotationOptions,
|
options: TokenizerAnnotationOptions,
|
||||||
): Promise<MergedToken[]> {
|
): Promise<MergedToken[]> {
|
||||||
if (!hasAnyAnnotationEnabled(options)) {
|
if (!hasAnyAnnotationEnabled(options)) {
|
||||||
return stripSubtitleAnnotationMetadata(tokens);
|
return stripSubtitleAnnotationMetadata(tokens, options);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!annotationStageModulePromise) {
|
if (!annotationStageModulePromise) {
|
||||||
@@ -179,7 +179,10 @@ async function applyAnnotationStage(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function stripSubtitleAnnotationMetadata(tokens: MergedToken[]): Promise<MergedToken[]> {
|
async function stripSubtitleAnnotationMetadata(
|
||||||
|
tokens: MergedToken[],
|
||||||
|
options: TokenizerAnnotationOptions,
|
||||||
|
): Promise<MergedToken[]> {
|
||||||
if (tokens.length === 0) {
|
if (tokens.length === 0) {
|
||||||
return tokens;
|
return tokens;
|
||||||
}
|
}
|
||||||
@@ -189,7 +192,7 @@ async function stripSubtitleAnnotationMetadata(tokens: MergedToken[]): Promise<M
|
|||||||
}
|
}
|
||||||
|
|
||||||
const annotationStage = await annotationStageModulePromise;
|
const annotationStage = await annotationStageModulePromise;
|
||||||
return tokens.map((token) => annotationStage.stripSubtitleAnnotationMetadata(token));
|
return tokens.map((token) => annotationStage.stripSubtitleAnnotationMetadata(token, options));
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createTokenizerDepsRuntime(
|
export function createTokenizerDepsRuntime(
|
||||||
@@ -683,25 +686,23 @@ async function parseWithYomitanInternalParser(
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
const normalizedSelectedTokens = normalizeSelectedYomitanTokens(
|
const normalizedSelectedTokens = normalizeSelectedYomitanTokens(
|
||||||
selectedTokens.map(
|
selectedTokens.map((token): MergedToken => {
|
||||||
(token): MergedToken => {
|
const posMetadata = getYomitanWordClassPosMetadata(token.wordClasses);
|
||||||
const posMetadata = getYomitanWordClassPosMetadata(token.wordClasses);
|
return {
|
||||||
return {
|
surface: token.surface,
|
||||||
surface: token.surface,
|
reading: token.reading,
|
||||||
reading: token.reading,
|
headword: token.headword,
|
||||||
headword: token.headword,
|
startPos: token.startPos,
|
||||||
startPos: token.startPos,
|
endPos: token.endPos,
|
||||||
endPos: token.endPos,
|
partOfSpeech: posMetadata.partOfSpeech,
|
||||||
partOfSpeech: posMetadata.partOfSpeech,
|
pos1: posMetadata.pos1,
|
||||||
pos1: posMetadata.pos1,
|
isMerged: true,
|
||||||
isMerged: true,
|
isKnown: false,
|
||||||
isKnown: false,
|
isNPlusOneTarget: false,
|
||||||
isNPlusOneTarget: false,
|
isNameMatch: token.isNameMatch ?? false,
|
||||||
isNameMatch: token.isNameMatch ?? false,
|
frequencyRank: token.frequencyRank,
|
||||||
frequencyRank: token.frequencyRank,
|
};
|
||||||
};
|
}),
|
||||||
},
|
|
||||||
),
|
|
||||||
);
|
);
|
||||||
|
|
||||||
if (deps.getYomitanGroupDebugEnabled?.() === true) {
|
if (deps.getYomitanGroupDebugEnabled?.() === true) {
|
||||||
|
|||||||
@@ -507,8 +507,11 @@ export function shouldExcludeTokenFromSubtitleAnnotations(token: MergedToken): b
|
|||||||
return sharedShouldExcludeTokenFromSubtitleAnnotations(token);
|
return sharedShouldExcludeTokenFromSubtitleAnnotations(token);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function stripSubtitleAnnotationMetadata(token: MergedToken): MergedToken {
|
export function stripSubtitleAnnotationMetadata(
|
||||||
return sharedStripSubtitleAnnotationMetadata(token);
|
token: MergedToken,
|
||||||
|
options: AnnotationStageOptions = {},
|
||||||
|
): MergedToken {
|
||||||
|
return sharedStripSubtitleAnnotationMetadata(token, options);
|
||||||
}
|
}
|
||||||
|
|
||||||
function computeTokenKnownStatus(
|
function computeTokenKnownStatus(
|
||||||
|
|||||||
@@ -310,8 +310,7 @@ function fillMissingPos1BySurfaceSequence(
|
|||||||
|
|
||||||
let cursor = 0;
|
let cursor = 0;
|
||||||
return tokens.map((token) => {
|
return tokens.map((token) => {
|
||||||
const hasCompletePosMetadata =
|
const hasCompletePosMetadata = token.pos1?.trim() && token.pos2?.trim() && token.pos3?.trim();
|
||||||
token.pos1?.trim() && token.pos2?.trim() && token.pos3?.trim();
|
|
||||||
if (hasCompletePosMetadata) {
|
if (hasCompletePosMetadata) {
|
||||||
return token;
|
return token;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -85,12 +85,16 @@ export function createBuildBindMpvMainEventHandlersMainDepsHandler(deps: {
|
|||||||
hasInitialPlaybackQuitOnDisconnectArg: () =>
|
hasInitialPlaybackQuitOnDisconnectArg: () =>
|
||||||
Boolean(
|
Boolean(
|
||||||
deps.appState.initialArgs?.managedPlayback ||
|
deps.appState.initialArgs?.managedPlayback ||
|
||||||
deps.appState.initialArgs?.jellyfinPlay ||
|
deps.appState.initialArgs?.jellyfinPlay ||
|
||||||
deps.appState.initialArgs?.youtubePlay,
|
deps.appState.initialArgs?.youtubePlay,
|
||||||
),
|
),
|
||||||
isOverlayRuntimeInitialized: () => deps.appState.overlayRuntimeInitialized,
|
isOverlayRuntimeInitialized: () => deps.appState.overlayRuntimeInitialized,
|
||||||
shouldQuitOnDisconnectWhenOverlayRuntimeInitialized: () =>
|
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(),
|
isQuitOnDisconnectArmed: () => deps.getQuitOnDisconnectArmed(),
|
||||||
scheduleQuitCheck: (callback: () => void) => deps.scheduleQuitCheck(callback),
|
scheduleQuitCheck: (callback: () => void) => deps.scheduleQuitCheck(callback),
|
||||||
isMpvConnected: () => Boolean(deps.appState.mpvClient?.connected),
|
isMpvConnected: () => Boolean(deps.appState.mpvClient?.connected),
|
||||||
|
|||||||
@@ -1347,7 +1347,8 @@ test('window resize allows primary hover pause from a real mouseenter over subti
|
|||||||
configurable: true,
|
configurable: true,
|
||||||
value: {
|
value: {
|
||||||
addEventListener: () => {},
|
addEventListener: () => {},
|
||||||
elementFromPoint: () => ctx.dom.subtitleContainer,
|
elementFromPoint: (x: number, y: number) =>
|
||||||
|
x === 120 && y === 240 ? ctx.dom.subtitleContainer : null,
|
||||||
querySelectorAll: () => [],
|
querySelectorAll: () => [],
|
||||||
body: {},
|
body: {},
|
||||||
},
|
},
|
||||||
@@ -1496,7 +1497,8 @@ test('pointer tracking enables overlay interaction as soon as the cursor reaches
|
|||||||
bucket.push(listener);
|
bucket.push(listener);
|
||||||
documentListeners.set(type, bucket);
|
documentListeners.set(type, bucket);
|
||||||
},
|
},
|
||||||
elementFromPoint: () => ctx.dom.subtitleContainer,
|
elementFromPoint: (x: number, y: number) =>
|
||||||
|
x === 120 && y === 240 ? ctx.dom.subtitleContainer : null,
|
||||||
querySelectorAll: () => [],
|
querySelectorAll: () => [],
|
||||||
body: {},
|
body: {},
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1069,10 +1069,7 @@ test('subtitle annotation CSS underlines JLPT tokens without changing token colo
|
|||||||
cssText,
|
cssText,
|
||||||
`#subtitleRoot .word.word-jlpt-n${level}::selection`,
|
`#subtitleRoot .word.word-jlpt-n${level}::selection`,
|
||||||
);
|
);
|
||||||
assert.ok(
|
assert.ok(jlptSelectionLockBlock.length > 0, `word-jlpt-n${level} selection lock should exist`);
|
||||||
jlptSelectionLockBlock.length > 0,
|
|
||||||
`word-jlpt-n${level} selection lock should exist`,
|
|
||||||
);
|
|
||||||
assert.match(
|
assert.match(
|
||||||
jlptSelectionLockBlock,
|
jlptSelectionLockBlock,
|
||||||
new RegExp(
|
new RegExp(
|
||||||
|
|||||||
Reference in New Issue
Block a user