mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-08-13 01:55:50 -07:00
fix(stats): fix duplicate-line cleanup edge cases
- Drain the full write queue before scanning, not just one batch, so pending burst rows aren't missed - Floor lookback-days before the positivity check so a sub-day value no longer collapses to a zero-day window - Let the parsed cue list override the streaming dedup heuristic wherever it covers a line - Reject combining --lifetime with --duplicate-lines and non-positive --lookback-days values - Widen the burst frame bound to catch heavier typesetting while still sparing longer-spaced runs - Keep the cleanup modal open until the user closes it so they can read the result before it unmounts
This commit is contained in:
@@ -147,6 +147,8 @@ subminer stats cleanup --duplicate-lines --dry-run --lookback-days 30
|
||||
subminer stats cleanup --duplicate-lines --lookback-days 30
|
||||
```
|
||||
|
||||
`--duplicate-lines` (short: `-d`) picks the cleanup mode, so it cannot be combined with `--lifetime`, and `--dry-run` and `--lookback-days <days>` only apply to it. Omitting `--lookback-days` scans all history; the value must be at least one day.
|
||||
|
||||
Runs never cross a session boundary, so rewatching an episode keeps both watches. Session telemetry (watch time, lines seen, tokens seen) and the rollups derived from it are left as recorded: they are cumulative samples taken during playback, and cannot be recomputed for sessions whose raw rows have since been pruned.
|
||||
|
||||
## Retention Defaults
|
||||
|
||||
@@ -96,6 +96,7 @@ subminer stats -b # Start/reuse the background stats daemon
|
||||
subminer stats -s # Stop the background stats daemon
|
||||
subminer stats cleanup # Backfill vocabulary metadata, prune stale rows
|
||||
subminer stats cleanup -d --dry-run # Preview cleanup of repeated typeset subtitle lines
|
||||
subminer stats cleanup -d --lookback-days 30 # Clean only lines recorded in the last 30 days
|
||||
subminer stats rebuild # Rebuild rollup data
|
||||
subminer doctor --refresh-known-words # Refresh the known-word cache
|
||||
subminer logs -e # Export a sanitized log ZIP and print its path
|
||||
|
||||
@@ -255,6 +255,30 @@ test('parseArgs rejects duplicate-line flags without the duplicate-lines mode',
|
||||
assert.match(error.stderr, /--dry-run and --lookback-days require --duplicate-lines/);
|
||||
});
|
||||
|
||||
test('parseArgs rejects combining lifetime and duplicate-line cleanup modes', () => {
|
||||
const error = withProcessExitIntercept(() => {
|
||||
parseArgs(['stats', 'cleanup', '--lifetime', '--duplicate-lines'], 'subminer', {});
|
||||
});
|
||||
|
||||
assert.equal(error.code, 1);
|
||||
assert.match(error.stderr, /Stats cleanup runs one mode at a time/);
|
||||
});
|
||||
|
||||
test('parseArgs rejects unusable lookback windows', () => {
|
||||
for (const value of ['0', '-5', 'soon']) {
|
||||
const error = withProcessExitIntercept(() => {
|
||||
parseArgs(
|
||||
['stats', 'cleanup', '--duplicate-lines', '--lookback-days', value],
|
||||
'subminer',
|
||||
{},
|
||||
);
|
||||
});
|
||||
|
||||
assert.equal(error.code, 1);
|
||||
assert.match(error.stderr, /--lookback-days must be a positive number of days/);
|
||||
}
|
||||
});
|
||||
|
||||
test('parseArgs rejects cleanup-only stats flags without cleanup action', () => {
|
||||
const error = withProcessExitIntercept(() => {
|
||||
parseArgs(['stats', '--vocab'], 'subminer', {});
|
||||
|
||||
@@ -1064,6 +1064,37 @@ describe('stats server API routes', () => {
|
||||
assert.deepEqual(seenOptions, { dryRun: true, lookbackDays: 30 });
|
||||
});
|
||||
|
||||
it('POST /api/stats/maintenance/duplicate-lines ignores a window shorter than a day', async () => {
|
||||
let seenOptions: unknown = null;
|
||||
const app = createStatsApp(
|
||||
createMockTracker({
|
||||
cleanupDuplicateSubtitleLines: async (options: unknown) => {
|
||||
seenOptions = options;
|
||||
return {
|
||||
dryRun: true,
|
||||
lookbackDays: null,
|
||||
scannedLines: 0,
|
||||
burstGroups: 0,
|
||||
removedLines: 0,
|
||||
removedWordOccurrences: 0,
|
||||
removedKanjiOccurrences: 0,
|
||||
samples: [],
|
||||
};
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const res = await app.request('/api/stats/maintenance/duplicate-lines', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ dryRun: true, lookbackDays: 0.5 }),
|
||||
});
|
||||
|
||||
assert.equal(res.status, 200);
|
||||
// Half a day must not floor to a zero-day window; it means no limit.
|
||||
assert.deepEqual(seenOptions, { dryRun: true, lookbackDays: null });
|
||||
});
|
||||
|
||||
it('POST /api/stats/maintenance/duplicate-lines treats a missing body as an apply over all history', async () => {
|
||||
let seenOptions: unknown = null;
|
||||
const app = createStatsApp(
|
||||
|
||||
@@ -601,14 +601,14 @@ export class ImmersionTrackerService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Collapse animation bursts that earlier versions recorded frame by frame. Pending
|
||||
* writes are flushed first so a burst that is still queued is scanned as stored rows
|
||||
* rather than surviving the cleanup and reappearing seconds later.
|
||||
* Collapse animation bursts that earlier versions recorded frame by frame. The whole
|
||||
* queue is drained first so a burst still waiting to be written is scanned as stored
|
||||
* rows rather than surviving the cleanup and landing a moment after it.
|
||||
*/
|
||||
async cleanupDuplicateSubtitleLines(
|
||||
options: DuplicateSubtitleLineCleanupOptions = {},
|
||||
): Promise<DuplicateSubtitleLineCleanupSummary> {
|
||||
this.flushNow();
|
||||
this.drainQueue();
|
||||
return cleanupDuplicateSubtitleLines(this.db, options);
|
||||
}
|
||||
|
||||
@@ -1816,6 +1816,24 @@ export class ImmersionTrackerService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Write out everything queued, not just the next batch.
|
||||
*
|
||||
* `flushNow` writes at most `batchSize` entries and does nothing at all while the write
|
||||
* lock is held, so a maintenance pass that runs straight after it can still be reading
|
||||
* a database that is missing rows. Each pass has to shrink the queue to continue: a
|
||||
* failed flush puts its batch back, and looping on that would never finish.
|
||||
*/
|
||||
private drainQueue(): void {
|
||||
while (this.queue.length > 0) {
|
||||
const pendingBefore = this.queue.length;
|
||||
this.flushNow();
|
||||
if (this.queue.length >= pendingBefore) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private flushSingle(write: QueuedWrite): void {
|
||||
executeQueuedWrite(write, this.preparedStatements);
|
||||
}
|
||||
|
||||
@@ -59,11 +59,12 @@ function seed(db: DatabaseSync, lines: SeedLine[]): void {
|
||||
|
||||
lines.forEach((line, index) => {
|
||||
const lineId = index + 1;
|
||||
const lineIndex = index + 1;
|
||||
const createdMs = line.createdMs ?? BASE_MS;
|
||||
insertLine.run(
|
||||
lineId,
|
||||
line.session,
|
||||
lineId,
|
||||
lineIndex,
|
||||
line.startMs,
|
||||
line.endMs,
|
||||
line.text,
|
||||
@@ -174,6 +175,38 @@ test('ordinary repeated dialogue survives', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('a long run of quarter-second frames is still a burst', () => {
|
||||
// Between the timing-only bound (0.1s) and the animation-frame bound (0.3s): heavier
|
||||
// typesetting lands here, and the run length is what makes it conclusive.
|
||||
const { db, dbPath } = createDb(karaokeFrames(1, '飛び上がる', 10_000, 6, 250));
|
||||
|
||||
try {
|
||||
const summary = cleanupDuplicateSubtitleLines(db);
|
||||
|
||||
assert.equal(summary.burstGroups, 1);
|
||||
assert.equal(summary.removedLines, 5);
|
||||
assert.equal(countLines(db), 1);
|
||||
assert.equal(wordFrequency(db), 1);
|
||||
} finally {
|
||||
db.close();
|
||||
cleanupDbPath(dbPath);
|
||||
}
|
||||
});
|
||||
|
||||
test('a run of frames longer than the animation bound survives', () => {
|
||||
const { db, dbPath } = createDb(karaokeFrames(1, '飛び上がる', 10_000, 6, 400));
|
||||
|
||||
try {
|
||||
const summary = cleanupDuplicateSubtitleLines(db);
|
||||
|
||||
assert.equal(summary.burstGroups, 0);
|
||||
assert.equal(countLines(db), 6);
|
||||
} finally {
|
||||
db.close();
|
||||
cleanupDbPath(dbPath);
|
||||
}
|
||||
});
|
||||
|
||||
test('a short run below the threshold survives', () => {
|
||||
const { db, dbPath } = createDb(karaokeFrames(1, '飛び上がる', 1_000, 4, 40));
|
||||
|
||||
|
||||
@@ -8,9 +8,16 @@
|
||||
*
|
||||
* Only timing is available here: the stored text has been stripped of ASS markup, so the
|
||||
* authoring evidence the file-level parser uses (`\t`, `\move`, karaoke timing, a
|
||||
* changing override signature) is long gone. The rule is therefore the strict,
|
||||
* metadata-free one -- a long run of identical, contiguous, short-lived lines inside a
|
||||
* single session -- and every bound is configurable so a cautious run can ask for more.
|
||||
* changing override signature) is long gone. What is left is a run of identical,
|
||||
* contiguous, short-lived lines inside a single session.
|
||||
*
|
||||
* The run has to be as long as the timing-only rule in `subtitle-cue-dedup` demands, but
|
||||
* each line may be as long as the animation-frame bound rather than the much tighter
|
||||
* timing-only one. Five or more repeats of the same text, each ending where the next
|
||||
* begins, is already conclusive on its own -- no dialogue does that -- and the tighter
|
||||
* bound would walk straight past the heavier typesetting that motivated this, where
|
||||
* frames sit nearer a quarter of a second. Both bounds are options, so a cautious run can
|
||||
* ask for more, and a dry run always reports before anything is removed.
|
||||
*
|
||||
* Scope: subtitle lines, their word/kanji occurrences, and the `imm_words`/`imm_kanji`
|
||||
* aggregates those occurrences feed. Session telemetry (`lines_seen`, `tokens_seen`) and
|
||||
|
||||
@@ -98,10 +98,12 @@ export function parseDuplicateLineCleanupBody(body: unknown): {
|
||||
} {
|
||||
const source = body && typeof body === 'object' ? (body as Record<string, unknown>) : {};
|
||||
const rawLookback = source.lookbackDays;
|
||||
const lookbackDays =
|
||||
typeof rawLookback === 'number' && Number.isFinite(rawLookback) && rawLookback > 0
|
||||
// Floor before the bounds check, or a fraction of a day arrives as a zero-day window.
|
||||
const wholeDays =
|
||||
typeof rawLookback === 'number' && Number.isFinite(rawLookback)
|
||||
? Math.floor(rawLookback)
|
||||
: null;
|
||||
const lookbackDays = wholeDays !== null && wholeDays >= 1 ? wholeDays : null;
|
||||
return { dryRun: source.dryRun === true, lookbackDays };
|
||||
}
|
||||
|
||||
|
||||
@@ -43,6 +43,23 @@ test('parsed cues keep separate lines that merely repeat', () => {
|
||||
assert.equal(recorded.length, 3);
|
||||
});
|
||||
|
||||
test('parsed cues outrank the streaming heuristic for short repeated cues', () => {
|
||||
// Long enough to trip the timing-only rule, but the parser saw these with full
|
||||
// lookahead and kept them, so every one of them is a line the sidebar shows.
|
||||
const cues: SubtitleCue[] = Array.from({ length: 8 }, (_, index) => ({
|
||||
startTime: 3 + index * 0.08,
|
||||
endTime: 3 + (index + 1) * 0.08,
|
||||
text: 'えっ',
|
||||
}));
|
||||
const gate = createSubtitleLineDedupGate({ getParsedCues: () => cues });
|
||||
|
||||
const recorded = cues.filter((cue) =>
|
||||
gate.shouldRecord({ text: cue.text, startSec: cue.startTime, endSec: cue.endTime }),
|
||||
);
|
||||
|
||||
assert.equal(recorded.length, 8);
|
||||
});
|
||||
|
||||
test('a line whose timing does not match any cue still records', () => {
|
||||
// A shifted track, an embedded sub nobody parsed: no match, no drop.
|
||||
const cues: SubtitleCue[] = [{ startTime: 10, endTime: 14, text: '飛び上がる' }];
|
||||
|
||||
@@ -169,10 +169,14 @@ export function createSubtitleLineDedupGate(
|
||||
return true;
|
||||
}
|
||||
|
||||
// The parsed cue list has the final say wherever it covers this line. Falling
|
||||
// through to the streaming heuristic would let it drop cues the parser looked at
|
||||
// with full lookahead and deliberately kept apart, which is the disagreement
|
||||
// between sidebar and stats this gate exists to prevent.
|
||||
const spans = lookupSpans(text);
|
||||
if (spans && isMergedAwayFrame(spans, sample.startSec)) {
|
||||
if (spans) {
|
||||
run = null;
|
||||
return false;
|
||||
return !isMergedAwayFrame(spans, sample.startSec);
|
||||
}
|
||||
|
||||
return advanceStreamingRun(text, sample);
|
||||
|
||||
@@ -43,7 +43,6 @@ export function DuplicateLineCleanup({ onClose, onCleaned }: DuplicateLineCleanu
|
||||
} else {
|
||||
setApplied(result);
|
||||
setPreview(null);
|
||||
onCleaned();
|
||||
}
|
||||
} catch (cause) {
|
||||
setError(cause instanceof Error ? cause.message : String(cause));
|
||||
@@ -51,9 +50,18 @@ export function DuplicateLineCleanup({ onClose, onCleaned }: DuplicateLineCleanu
|
||||
setBusy(null);
|
||||
}
|
||||
},
|
||||
[lookbackDays, onCleaned],
|
||||
[lookbackDays],
|
||||
);
|
||||
|
||||
// Reloading the vocabulary tables unmounts this modal along with the rest of the tab,
|
||||
// so it waits for the user to close: they get to read what was removed first.
|
||||
const close = useCallback(() => {
|
||||
if (applied && applied.removedLines > 0) {
|
||||
onCleaned();
|
||||
}
|
||||
onClose();
|
||||
}, [applied, onCleaned, onClose]);
|
||||
|
||||
const result = applied ?? preview;
|
||||
const nothingToDo = preview !== null && preview.removedLines === 0;
|
||||
|
||||
@@ -63,7 +71,7 @@ export function DuplicateLineCleanup({ onClose, onCleaned }: DuplicateLineCleanu
|
||||
type="button"
|
||||
aria-label="Close duplicate line cleanup"
|
||||
className="absolute inset-0 bg-ctp-crust/70 backdrop-blur-[2px]"
|
||||
onClick={onClose}
|
||||
onClick={close}
|
||||
/>
|
||||
<div className="absolute inset-x-0 top-1/2 mx-auto max-w-xl -translate-y-1/2 rounded-xl border border-ctp-surface1 bg-ctp-mantle shadow-2xl">
|
||||
<div className="flex items-center justify-between border-b border-ctp-surface1 px-5 py-4">
|
||||
@@ -71,7 +79,7 @@ export function DuplicateLineCleanup({ onClose, onCleaned }: DuplicateLineCleanu
|
||||
<button
|
||||
type="button"
|
||||
className="rounded-md border border-ctp-surface2 px-3 py-1.5 text-xs font-medium text-ctp-subtext0 transition hover:border-ctp-blue hover:text-ctp-blue"
|
||||
onClick={onClose}
|
||||
onClick={close}
|
||||
>
|
||||
Close
|
||||
</button>
|
||||
|
||||
Reference in New Issue
Block a user