mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-30 06:12:06 -07:00
Fix AniList token persistence and AVIF timing
- Defer AniList setup prompts until app-ready and reuse stored tokens - Add AVIF lead-in padding so motion stays aligned with sentence audio
This commit is contained in:
@@ -0,0 +1,68 @@
|
||||
---
|
||||
id: TASK-253
|
||||
title: Fix animated AVIF lead-in alignment with sentence audio
|
||||
status: Done
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-30 01:59'
|
||||
updated_date: '2026-03-30 02:03'
|
||||
labels: []
|
||||
dependencies: []
|
||||
references:
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/anki-integration/animated-image-sync.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/anki-integration.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/core/services/stats-server.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/media-generator.ts
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
Animated AVIF cards currently freeze only for the existing word-audio duration. Because generated sentence audio starts with configured audio padding before the spoken subtitle begins, animation motion can begin early instead of lining up with the spoken sentence. Update the shared lead-in calculation so animated motion begins when sentence speech begins after the chosen word audio finishes.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 Animated AVIF lead-in calculation includes both the chosen word-audio duration and the generated sentence-audio start offset so motion begins with spoken sentence audio
|
||||
- [x] #2 Shared animated-image sync behavior is applied consistently across the Anki note update, card creation, and stats server media-generation paths
|
||||
- [x] #3 Regression tests cover the corrected lead-in timing calculation and fail before the fix
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
Approved plan:
|
||||
1. Add a failing unit test proving animated-image lead-in must include sentence-audio start offset in addition to chosen word-audio duration.
|
||||
2. Update shared animated-image lead-in resolution to add the configured sentence-audio offset used by generated sentence audio.
|
||||
3. Thread the shared calculation through note update, card creation, and stats-server generation paths without duplicating timing logic.
|
||||
4. Run targeted tests first, then the relevant fast verification lane for touched files.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
User approved implementation on 2026-03-29 local time. Root cause: lead-in omitted sentence-audio padding offset, so AVIF motion began before spoken sentence audio.
|
||||
|
||||
Implemented shared animated-image lead-in fix in src/anki-integration/animated-image-sync.ts by adding the same sentence-audio start offset used by generated audio (`audioPadding`) after summing the chosen word-audio durations.
|
||||
|
||||
Added regression coverage in src/anki-integration/animated-image-sync.test.ts for explicit `audioPadding` lead-in alignment and kept the zero-padding case covered.
|
||||
|
||||
Verification passed: `bun test src/anki-integration/animated-image-sync.test.ts src/anki-integration/note-update-workflow.test.ts src/media-generator.test.ts`, `bun run typecheck`, `bun run test:fast`, `bun run test:env`, `bun run build`, `bun run test:smoke:dist`.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Fixed animated AVIF lead-in alignment so motion starts when the spoken sentence starts, not at the padded beginning of the generated sentence-audio clip. The shared resolver in `src/anki-integration/animated-image-sync.ts` now adds the configured/default `audioPadding` offset after summing the selected word-audio durations, which keeps note update, card creation, and stats-server generation paths aligned through the same logic.
|
||||
|
||||
Added regression coverage in `src/anki-integration/animated-image-sync.test.ts` for both zero-padding and explicit padding cases to prove the lead-in math matches sentence-audio timing.
|
||||
|
||||
Verification:
|
||||
- `bun test src/anki-integration/animated-image-sync.test.ts src/anki-integration/note-update-workflow.test.ts src/media-generator.test.ts`
|
||||
- `bun run typecheck`
|
||||
- `bun run test:fast`
|
||||
- `bun run test:env`
|
||||
- `bun run build`
|
||||
- `bun run test:smoke:dist`
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -0,0 +1,59 @@
|
||||
---
|
||||
id: TASK-254
|
||||
title: Fix AniList token persistence when safe storage is unavailable
|
||||
status: Done
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-30 02:10'
|
||||
updated_date: '2026-03-30 02:20'
|
||||
labels:
|
||||
- bug
|
||||
- anilist
|
||||
dependencies: []
|
||||
references:
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/core/services/anilist/anilist-token-store.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/main/runtime/anilist-setup.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/main/runtime/anilist-token-refresh.ts
|
||||
priority: high
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
AniList login currently appears to succeed during setup, but some environments cannot persist the token because Electron safeStorage is unavailable or unusable. On the next app start, AniList tracking cannot load the token and re-prompts the user to set up AniList again. Align AniList token persistence with the intended login UX so a token the user already saved is reused on later sessions.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 Saved encrypted AniList token is reused on app-ready startup without reopening setup.
|
||||
- [x] #2 AniList startup no longer attempts to open the setup BrowserWindow before Electron is ready.
|
||||
- [x] #3 AniList auth/runtime tests cover stored-token reuse and the missing-token startup path that previously triggered pre-ready setup attempts.
|
||||
- [x] #4 AniList token storage remains encrypted-only; no plaintext fallback is introduced.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Add regression tests for AniList startup auth refresh so a stored encrypted token is reused without opening setup, and for the missing-token path so setup opening is deferred safely until the app can actually show a window.
|
||||
2. Update AniList startup/auth runtime to separate token resolution from setup-window prompting, and gate prompting on app readiness instead of attempting BrowserWindow creation during early startup.
|
||||
3. Preserve encrypted-only storage semantics in anilist-token-store; do not add plaintext fallback. If stored-token load fails, keep logging/diagnostics intact.
|
||||
4. Run targeted AniList runtime/token tests, then summarize root cause and verification results.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Investigated AniList auth persistence flow. Current setup path treats callback token as saved even when anilist-token-store refuses persistence because safeStorage is unavailable. Jellyfin token store already uses plaintext fallback in this environment class, which is a likely model for the AniList fix.
|
||||
|
||||
Confirmed from local logs that safeStorage was explicitly unavailable on 2026-03-23 due macOS Keychain lookup failure with NSOSStatusErrorDomain Code=-128 userCanceledErr. Current environment also has an encrypted AniList token file at /Users/sudacode/.config/SubMiner/anilist-token-store.json updated 2026-03-29 18:49, so safeStorage did work recently for save. Repeated AniList setup prompts on 2026-03-29/30 correlate more strongly with startup auth flow deciding no token is available and opening setup immediately; logs show repeated 'Loaded AniList manual token entry page' and several 'Failed to refresh AniList client secret state during startup' errors with 'Cannot create BrowserWindow before app is ready'. No recent log lines indicate safeStorage.isEncryptionAvailable() false after 2026-03-23.
|
||||
|
||||
Implemented encrypted-only startup fix by adding an allowSetupPrompt control to AniList token refresh and disabling setup-window prompting for the early pre-ready startup refresh in main.ts. App-ready reloadConfig still performs the normal prompt-capable refresh after Electron is ready. Added regression tests for stored-token reuse and prompt suppression when startup explicitly disables prompting.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Root cause was a redundant early AniList auth refresh during startup. The app refreshed AniList auth once before Electron was ready and again during app-ready config reload. When the early refresh could not resolve a token, it tried to open the AniList setup window immediately, which produced the observed 'Cannot create BrowserWindow before app is ready' failures and repeated setup prompts. The fix keeps token storage encrypted-only, teaches AniList auth refresh to optionally suppress setup-window prompting, and uses that suppression for the early startup refresh. App-ready startup still performs the normal prompt-capable refresh once Electron is ready, so saved encrypted tokens are reused without reopening setup and missing-token setup only happens at a safe point. Verified with targeted AniList auth tests, typecheck, test:fast, test:env, build, and test:smoke:dist.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -19,6 +19,7 @@ test('resolveAnimatedImageLeadInSeconds sums configured word audio durations for
|
||||
media: {
|
||||
imageType: 'avif',
|
||||
syncAnimatedImageToWordAudio: true,
|
||||
audioPadding: 0,
|
||||
},
|
||||
},
|
||||
noteInfo: {
|
||||
@@ -49,6 +50,46 @@ test('resolveAnimatedImageLeadInSeconds sums configured word audio durations for
|
||||
assert.equal(leadInSeconds, 1.25);
|
||||
});
|
||||
|
||||
test('resolveAnimatedImageLeadInSeconds adds sentence audio padding to word audio duration', async () => {
|
||||
const leadInSeconds = await resolveAnimatedImageLeadInSeconds({
|
||||
config: {
|
||||
fields: {
|
||||
audio: 'ExpressionAudio',
|
||||
},
|
||||
media: {
|
||||
imageType: 'avif',
|
||||
syncAnimatedImageToWordAudio: true,
|
||||
audioPadding: 0.5,
|
||||
},
|
||||
},
|
||||
noteInfo: {
|
||||
noteId: 42,
|
||||
fields: {
|
||||
ExpressionAudio: {
|
||||
value: '[sound:word.mp3][sound:alt.ogg]',
|
||||
},
|
||||
},
|
||||
},
|
||||
resolveConfiguredFieldName: (noteInfo, ...preferredNames) => {
|
||||
for (const preferredName of preferredNames) {
|
||||
if (!preferredName) continue;
|
||||
const resolved = Object.keys(noteInfo.fields).find(
|
||||
(fieldName) => fieldName.toLowerCase() === preferredName.toLowerCase(),
|
||||
);
|
||||
if (resolved) return resolved;
|
||||
}
|
||||
return null;
|
||||
},
|
||||
retrieveMediaFileBase64: async (filename) =>
|
||||
filename === 'word.mp3' ? 'd29yZA==' : filename === 'alt.ogg' ? 'YWx0' : '',
|
||||
probeAudioDurationSeconds: async (_buffer, filename) =>
|
||||
filename === 'word.mp3' ? 0.41 : filename === 'alt.ogg' ? 0.84 : null,
|
||||
logWarn: () => undefined,
|
||||
});
|
||||
|
||||
assert.equal(leadInSeconds, 1.75);
|
||||
});
|
||||
|
||||
test('resolveAnimatedImageLeadInSeconds falls back to zero when sync is disabled', async () => {
|
||||
const leadInSeconds = await resolveAnimatedImageLeadInSeconds({
|
||||
config: {
|
||||
|
||||
@@ -39,6 +39,14 @@ function shouldSyncAnimatedImageToWordAudio(config: Pick<AnkiConnectConfig, 'med
|
||||
return config.media?.imageType === 'avif' && config.media?.syncAnimatedImageToWordAudio !== false;
|
||||
}
|
||||
|
||||
function resolveSentenceAudioStartOffsetSeconds(config: Pick<AnkiConnectConfig, 'media'>): number {
|
||||
const configuredPadding = config.media?.audioPadding;
|
||||
if (typeof configuredPadding === 'number' && Number.isFinite(configuredPadding)) {
|
||||
return configuredPadding;
|
||||
}
|
||||
return DEFAULT_ANKI_CONNECT_CONFIG.media.audioPadding;
|
||||
}
|
||||
|
||||
export async function probeAudioDurationSeconds(
|
||||
buffer: Buffer,
|
||||
filename: string,
|
||||
@@ -127,5 +135,5 @@ export async function resolveAnimatedImageLeadInSeconds<TNoteInfo extends NoteIn
|
||||
totalLeadInSeconds += durationSeconds;
|
||||
}
|
||||
|
||||
return totalLeadInSeconds;
|
||||
return totalLeadInSeconds + resolveSentenceAudioStartOffsetSeconds(config);
|
||||
}
|
||||
|
||||
@@ -2591,6 +2591,7 @@ const {
|
||||
|
||||
function refreshAnilistClientSecretStateIfEnabled(options?: {
|
||||
force?: boolean;
|
||||
allowSetupPrompt?: boolean;
|
||||
}): Promise<string | null> {
|
||||
if (!isAnilistTrackingEnabled(getResolvedConfig())) {
|
||||
return Promise.resolve(null);
|
||||
@@ -4480,7 +4481,10 @@ const shouldUseMinimalStartup = startupModeFlags.shouldUseMinimalStartup;
|
||||
const shouldSkipHeavyStartup = startupModeFlags.shouldSkipHeavyStartup;
|
||||
if (!appState.initialArgs || (!shouldUseMinimalStartup && !shouldSkipHeavyStartup)) {
|
||||
if (isAnilistTrackingEnabled(getResolvedConfig())) {
|
||||
void refreshAnilistClientSecretStateIfEnabled({ force: true }).catch((error) => {
|
||||
void refreshAnilistClientSecretStateIfEnabled({
|
||||
force: true,
|
||||
allowSetupPrompt: false,
|
||||
}).catch((error) => {
|
||||
logger.error('Failed to refresh AniList client secret state during startup', error);
|
||||
});
|
||||
anilistStateRuntime.refreshRetryQueueState();
|
||||
|
||||
@@ -82,7 +82,42 @@ test('refresh handler prefers cached token when not forced', async () => {
|
||||
assert.equal(loadCalls, 0);
|
||||
});
|
||||
|
||||
test('refresh handler falls back to stored token then opens setup when missing', async () => {
|
||||
test('refresh handler falls back to stored token without opening setup', async () => {
|
||||
let cached: string | null = null;
|
||||
let opened = false;
|
||||
let openCalls = 0;
|
||||
const states: Array<{ status: string; source: string }> = [];
|
||||
const refresh = createRefreshAnilistClientSecretStateHandler({
|
||||
getResolvedConfig: () => ({ anilist: { accessToken: '' } }),
|
||||
isAnilistTrackingEnabled: () => true,
|
||||
getCachedAccessToken: () => cached,
|
||||
setCachedAccessToken: (token) => {
|
||||
cached = token;
|
||||
},
|
||||
saveStoredToken: () => {},
|
||||
loadStoredToken: () => ' stored-token ',
|
||||
setClientSecretState: (state) => {
|
||||
states.push({ status: state.status, source: state.source });
|
||||
},
|
||||
getAnilistSetupPageOpened: () => opened,
|
||||
setAnilistSetupPageOpened: (next) => {
|
||||
opened = next;
|
||||
},
|
||||
openAnilistSetupWindow: () => {
|
||||
openCalls += 1;
|
||||
},
|
||||
now: () => 400,
|
||||
});
|
||||
|
||||
const token = await refresh({ force: true });
|
||||
assert.equal(token, 'stored-token');
|
||||
assert.equal(cached, 'stored-token');
|
||||
assert.equal(opened, false);
|
||||
assert.equal(openCalls, 0);
|
||||
assert.deepEqual(states, [{ status: 'resolved', source: 'stored' }]);
|
||||
});
|
||||
|
||||
test('refresh handler opens setup when missing token and prompting allowed', async () => {
|
||||
let cached: string | null = null;
|
||||
let opened = false;
|
||||
let openCalls = 0;
|
||||
@@ -111,3 +146,44 @@ test('refresh handler falls back to stored token then opens setup when missing',
|
||||
assert.equal(cached, null);
|
||||
assert.equal(openCalls, 1);
|
||||
});
|
||||
|
||||
test('refresh handler skips setup open when missing token and prompting disabled', async () => {
|
||||
let cached: string | null = null;
|
||||
let opened = false;
|
||||
let openCalls = 0;
|
||||
const states: Array<{ status: string; source: string; message: string | null }> = [];
|
||||
const refresh = createRefreshAnilistClientSecretStateHandler({
|
||||
getResolvedConfig: () => ({ anilist: { accessToken: '' } }),
|
||||
isAnilistTrackingEnabled: () => true,
|
||||
getCachedAccessToken: () => cached,
|
||||
setCachedAccessToken: (token) => {
|
||||
cached = token;
|
||||
},
|
||||
saveStoredToken: () => {},
|
||||
loadStoredToken: () => '',
|
||||
setClientSecretState: (state) => {
|
||||
states.push({ status: state.status, source: state.source, message: state.message });
|
||||
},
|
||||
getAnilistSetupPageOpened: () => opened,
|
||||
setAnilistSetupPageOpened: (next) => {
|
||||
opened = next;
|
||||
},
|
||||
openAnilistSetupWindow: () => {
|
||||
openCalls += 1;
|
||||
},
|
||||
now: () => 500,
|
||||
});
|
||||
|
||||
const token = await refresh({ force: true, allowSetupPrompt: false });
|
||||
assert.equal(token, null);
|
||||
assert.equal(cached, null);
|
||||
assert.equal(opened, false);
|
||||
assert.equal(openCalls, 0);
|
||||
assert.deepEqual(states, [
|
||||
{
|
||||
status: 'error',
|
||||
source: 'none',
|
||||
message: 'cannot authenticate without anilist.accessToken',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -27,7 +27,10 @@ export function createRefreshAnilistClientSecretStateHandler<
|
||||
openAnilistSetupWindow: () => void;
|
||||
now: () => number;
|
||||
}) {
|
||||
return async (options?: { force?: boolean }): Promise<string | null> => {
|
||||
return async (options?: {
|
||||
force?: boolean;
|
||||
allowSetupPrompt?: boolean;
|
||||
}): Promise<string | null> => {
|
||||
const resolved = deps.getResolvedConfig();
|
||||
const now = deps.now();
|
||||
if (!deps.isAnilistTrackingEnabled(resolved)) {
|
||||
@@ -87,7 +90,11 @@ export function createRefreshAnilistClientSecretStateHandler<
|
||||
resolvedAt: null,
|
||||
errorAt: now,
|
||||
});
|
||||
if (deps.isAnilistTrackingEnabled(resolved) && !deps.getAnilistSetupPageOpened()) {
|
||||
if (
|
||||
options?.allowSetupPrompt !== false &&
|
||||
deps.isAnilistTrackingEnabled(resolved) &&
|
||||
!deps.getAnilistSetupPageOpened()
|
||||
) {
|
||||
deps.openAnilistSetupWindow();
|
||||
}
|
||||
return null;
|
||||
|
||||
Reference in New Issue
Block a user