mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-10 16:19:24 -07:00
Fix subtitle-cycle OSD labels and harden quality gates
- Resolve J/Shift+J subtitle-cycle OSD text via track-list labels and property expansion - Add mpv proxy OSD runtime + regression coverage across IPC/main/runtime paths - Scope `make pretty` to maintained source files and add scoped Prettier script - Align release workflow with cache/install order and explicit TypeScript check - Clean up duplicate submodule entry and remove checked-in docs/plans artifacts
This commit is contained in:
@@ -4,15 +4,15 @@ title: Index AniList character alternative names in the character dictionary
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: '2026-03-07 00:00'
|
||||
updated_date: '2026-03-07 00:00'
|
||||
updated_date: '2026-03-08 00:11'
|
||||
labels:
|
||||
- dictionary
|
||||
- anilist
|
||||
priority: high
|
||||
dependencies: []
|
||||
references:
|
||||
- /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.ts
|
||||
- /home/sudacode/projects/japanese/SubMiner/src/main/character-dictionary-runtime.test.ts
|
||||
- src/main/character-dictionary-runtime.ts
|
||||
- src/main/character-dictionary-runtime.test.ts
|
||||
priority: high
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
---
|
||||
id: TASK-111
|
||||
title: Fix subtitle-cycle OSD labels for J keybindings
|
||||
status: Done
|
||||
assignee:
|
||||
- Codex
|
||||
created_date: '2026-03-07 23:45'
|
||||
updated_date: '2026-03-08 00:06'
|
||||
labels: []
|
||||
dependencies: []
|
||||
references:
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/core/services/ipc-command.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/core/services/mpv.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/core/services/ipc-command.test.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/core/services/mpv-control.test.ts
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
When cycling subtitle tracks with the default J/Shift+J keybindings, the mpv OSD currently shows raw template text like `${sid}` instead of a resolved subtitle label. Update the keybinding OSD behavior so users see the active subtitle selection clearly when cycling tracks, and ensure placeholder-based OSD messages sent through the mpv client API render correctly.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 Pressing the primary subtitle cycle keybinding shows a resolved subtitle label on the OSD instead of a raw `${sid}` placeholder.
|
||||
- [x] #2 Pressing the secondary subtitle cycle keybinding shows a resolved subtitle label on the OSD instead of a raw `${secondary-sid}` placeholder.
|
||||
- [x] #3 Proxy OSD messages that rely on mpv property expansion render resolved values when sent through the mpv client API.
|
||||
- [x] #4 Regression tests cover the subtitle-cycle OSD behavior and the placeholder-expansion OSD path.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Add focused failing tests for subtitle-cycle OSD labels and mpv placeholder-expansion behavior.
|
||||
2. Update the IPC mpv command handler to resolve primary and secondary subtitle track labels from mpv `track-list` data after cycling subtitle tracks.
|
||||
3. Update the mpv OSD runtime path so placeholder-based `show-text` messages sent through the client API opt into property expansion.
|
||||
4. Run focused tests, then the relevant core test lane, and record results in the task notes.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Initial triage: `ipc-command.ts` emits raw `${sid}`/`${secondary-sid}` placeholder strings, and `showMpvOsdRuntime` sends `show-text` via mpv client API without enabling property expansion.
|
||||
|
||||
User approved implementation plan on 2026-03-07.
|
||||
|
||||
Implementation: proxy mpv command OSD now supports an async resolver so subtitle track cycling can show human-readable labels instead of raw `${sid}` placeholders.
|
||||
|
||||
Implementation: `showMpvOsdRuntime` now prefixes placeholder-based messages with mpv client-api `expand-properties`, which fixes raw `${...}` OSD output for subtitle delay/position messages.
|
||||
|
||||
Testing: `bun test src/core/services/ipc-command.test.ts src/core/services/mpv-control.test.ts src/main/runtime/mpv-proxy-osd.test.ts src/main/runtime/ipc-mpv-command-main-deps.test.ts src/main/runtime/ipc-bridge-actions.test.ts src/main/runtime/ipc-bridge-actions-main-deps.test.ts src/main/runtime/composers/ipc-runtime-composer.test.ts` passed.
|
||||
|
||||
Testing: `bun x tsc --noEmit` passed.
|
||||
|
||||
Testing: `bun run test:core:src` passed (423 pass, 6 skip, 0 fail).
|
||||
|
||||
Docs: no update required because no checked-in docs or help text describe the J/Shift+J OSD output behavior.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Fixed subtitle-cycle OSD handling for the default J/Shift+J keybindings. The IPC mpv command path now supports resolving proxy OSD text asynchronously, and the main-runtime resolver reads mpv `track-list` state so primary and secondary subtitle cycling show human-readable track labels instead of raw `${sid}` / `${secondary-sid}` placeholders.
|
||||
|
||||
Also fixed the lower-level mpv OSD transport so placeholder-based `show-text` messages sent through the client API opt into `expand-properties`. That preserves existing template-based OSD messages like subtitle delay and subtitle position without leaking the raw `${...}` syntax.
|
||||
|
||||
Added regression coverage for the async proxy OSD path, the placeholder-expansion `showMpvOsdRuntime` path, and the runtime subtitle-track label resolver. Verification run: `bun x tsc --noEmit`; focused mpv/IPC tests; and the maintained `bun run test:core:src` lane (423 pass, 6 skip, 0 fail).
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -0,0 +1,61 @@
|
||||
---
|
||||
id: TASK-112
|
||||
title: Address Claude review items on PR 15
|
||||
status: Done
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-08 00:11'
|
||||
updated_date: '2026-03-08 00:12'
|
||||
labels:
|
||||
- pr-review
|
||||
- ci
|
||||
dependencies: []
|
||||
references:
|
||||
- .github/workflows/release.yml
|
||||
- .github/workflows/ci.yml
|
||||
- .gitmodules
|
||||
- >-
|
||||
backlog/tasks/task-101 -
|
||||
Index-AniList-character-alternative-names-in-the-character-dictionary.md
|
||||
priority: medium
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
Review Claude's PR feedback on PR #15, implement only the technically valid fixes on the current branch, and document which comments are non-actionable or already acceptable.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 Validated Claude's concrete PR review items against current branch state and repo conventions
|
||||
- [x] #2 Implemented the accepted fixes with regression coverage or verification where applicable
|
||||
- [x] #3 Documented which review items are non-blocking or intentionally left unchanged
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Validate each Claude review item against current branch files and repo workflow.
|
||||
2. Patch release quality-gate to match CI ordering and add explicit typecheck.
|
||||
3. Remove duplicate .gitmodules stanza and normalize the TASK-101 reference path through Backlog MCP.
|
||||
4. Run relevant verification for workflow/config metadata changes and record which review items remain non-actionable.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
User asked to address Claude PR comments on PR #15 and assess whether any action items remain. Treat review suggestions skeptically; only fix validated defects.
|
||||
|
||||
Validated Claude's five review items. Fixed release workflow ordering/typecheck, removed the duplicate .gitmodules entry, and normalized TASK-101 references to repo-relative paths via Backlog MCP.
|
||||
|
||||
Left the vendor/subminer-yomitan branch-pin suggestion unchanged. The committed submodule SHA already controls reproducibility; adding a branch would only affect update ergonomics and was not required to address a concrete defect.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Validated Claude's PR #15 review summary against the current branch and applied the actionable fixes. In `.github/workflows/release.yml`, the release `quality-gate` job now restores the dependency cache before installation, no longer installs twice, and runs `bun run typecheck` before the fast test suite to match CI expectations. In `.gitmodules`, removed the duplicate `vendor/yomitan-jlpt-vocab` stanza with the conflicting duplicate path. Through Backlog MCP, updated `TASK-101` references from an absolute local path to repo-relative paths so the task metadata is portable across contributors.
|
||||
|
||||
Verification: `git diff --check`, `git config -f .gitmodules --get-regexp '^submodule\..*\.path$'`, `bun run typecheck`, and `bun run test:fast` all passed. `bun run format:check` still fails on many pre-existing unrelated files already present on the branch, including multiple backlog task files and existing source/docs files; this review patch did not attempt a repo-wide formatting sweep.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -0,0 +1,59 @@
|
||||
---
|
||||
id: TASK-113
|
||||
title: Scope make pretty to maintained source files
|
||||
status: Done
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-08 00:20'
|
||||
updated_date: '2026-03-08 00:22'
|
||||
labels:
|
||||
- tooling
|
||||
- formatting
|
||||
dependencies: []
|
||||
references:
|
||||
- Makefile
|
||||
- package.json
|
||||
priority: medium
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
Change the `make pretty` workflow so it formats only the maintained source/config files we intentionally keep under Prettier, instead of sweeping backlog/docs/generated content across the whole repository.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 `make pretty` formats only the approved maintained source/config paths
|
||||
- [x] #2 The allowlist is reusable for check/write flows instead of duplicating path logic
|
||||
- [x] #3 Verification shows the scoped formatting command targets the intended files without touching backlog or vendored content
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Inspect current Prettier config/ignore behavior and keep the broad repo-wide format command unchanged.
|
||||
2. Add a reusable scoped Prettier script that targets maintained source/config paths only.
|
||||
3. Update `make pretty` to call the scoped script.
|
||||
4. Verify the scoped command resolves only intended files and does not traverse backlog or vendor paths.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
User approved the allowlist approach: keep repo-wide `format` intact, make `make pretty` use a maintained-path formatter scope.
|
||||
|
||||
Added `scripts/prettier-scope.sh` as the single allowlist for scoped Prettier paths and wired `format:src` / `format:check:src` to it.
|
||||
|
||||
Updated `make pretty` to call `bun run format:src`. Verified with `make -n pretty` and shell tracing that the helper only targets the maintained allowlist and does not traverse `backlog/` or `vendor/`.
|
||||
|
||||
Excluded `Makefile` and `.prettierignore` from the allowlist after verification showed Prettier cannot infer parsers for them.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Scoped the repo's day-to-day formatting entrypoint without changing the existing broad repo-wide Prettier scripts. Added `scripts/prettier-scope.sh` as the shared allowlist for maintained source/config paths (`.github`, `build`, `launcher`, `scripts`, `src`, plus selected root JSON config files), added `format:src` and `format:check:src` in `package.json`, and updated `make pretty` to run the scoped formatter.
|
||||
|
||||
Verification: `make -n pretty` now resolves to `bun run format:src`. `bash -n scripts/prettier-scope.sh` passed, and shell-traced `bash -x scripts/prettier-scope.sh --check` confirmed the exact allowlist passed to Prettier. `bun run format:check:src` fails only because existing files inside the allowed source scope are not currently formatted; it no longer touches `backlog/` or `vendor/`.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
Reference in New Issue
Block a user