From f227d7742d64cbedde58a9a7c1fc01c8dc3ae9d4 Mon Sep 17 00:00:00 2001 From: wine-fall <62830944+wine-fall@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:20:22 +0800 Subject: [PATCH 1/3] feat(music): a song found on NetEase plays from YouTube when it can [spec 14] Finding a song and playing it are two decisions, and only the first is the model's. NetEase's CDN measured ~59 KB/s here against the ~1 Mbps FLAC yt-dlp picks for it, so a Chinese track stuttered straight off m801.music.126.net where the same song on YouTube would not (related: #272, which this routes around and does not close). submit_pick now relocates the pick before it resolves: walk the play order above the ref's own catalogue, over catalogues mounted and still open in this task, take the first hit whose title contains the submitted one and whose length is within 20 s of what the candidate stated, and resolve + probe it exactly as the original path would. Every failure falls to the next catalogue and then back to the ref the model actually submitted, so relocation can only make a pick faster, never lose one. The prompt stops steering the search at sources and says to find the best song. MURMUR_PLAY_ORDER sets the order (default youtube,bilibili,qqmusic,netease); one `music.relocate` line per attempt lands in the dev log. Verified at the real boundary: a NetEase ref through the real yt-dlp provider and ffmpeg probe finished on a googlevideo stream in 5.3 s, with `music.relocate from=netease to=youtube ok` in the log. Co-Authored-By: Claude Opus 5 --- specs/spec14/14-listening-taste.md | 96 +++++++++++++++++- src/app.ts | 2 + src/config.ts | 19 ++++ src/contracts.ts | 6 ++ src/music/music-programmer.ts | 5 +- src/music/music-tools.ts | 150 +++++++++++++++++++++++------ src/prompts/music.ts | 7 +- test/config.test.ts | 16 +++ test/fakes.ts | 8 +- test/music-relocate.test.ts | 141 +++++++++++++++++++++++++++ test/music-taste.test.ts | 4 +- 11 files changed, 414 insertions(+), 40 deletions(-) create mode 100644 test/music-relocate.test.ts diff --git a/specs/spec14/14-listening-taste.md b/specs/spec14/14-listening-taste.md index 5e7c443..7f77724 100644 --- a/specs/spec14/14-listening-taste.md +++ b/specs/spec14/14-listening-taste.md @@ -1072,6 +1072,81 @@ scores 0, the order falls back to newest first, and the render is what it was before this section existed: an unmatched pick, a source with no ledger and a silent moment all degrade by the same path, not by a special case. +### 2.13 Play-source preference — found anywhere, played from the fastest + +Where a song is **found** and where it **plays from** are two decisions, and +only the first belongs to the model. NetEase's CDN delivers ~59 KB/s to a +developer machine here while yt-dlp's `bestaudio` picks its FLAC (~1 Mbps), +so a NetEase pick stutters that the same song on YouTube or Bilibili would +not (measured 2026-09-20 on one Chinese track; related: issue #272, NetEase +resolve slowness — this routes around it and does not fix it). + +So after `submit_pick` has a ref and before the resolve, code — never the +model — tries to **relocate** the pick to a faster catalogue: + +```ts +playOrder: readonly ('youtube' | 'bilibili' | 'qqmusic' | 'netease')[] // default in that order +``` + +**When relocation is skipped**, and the original ref goes straight down +today's path: + +- the ref is a **segment** ref (`parseSegmentRef` yields a segment) — a + chapter of one specific upload has no equivalent elsewhere; +- **no title** was submitted — there is nothing to match a hit against; +- the ref's own catalogue is already **top-ranked** among the catalogues open + in this task. A ref whose host is unknown (or is YouTube) counts as + `youtube`; a `channels` pick is a YouTube or Bilibili upload and is read by + its host like any other ref. + +**Otherwise**, walk `playOrder` from the top down to (excluding) the ref's own +catalogue, visiting only catalogues that are **mounted and still open** in +this task. For each: + +1. `provider.search(`${artist ?? ''} ${title}`.trim(), 5, catalogue)`; +2. take the **first** hit whose folded title contains the submitted folded + title (or the reverse) **and** whose `durationS` is within **20 s** of the + length the original candidate stated (no stated length → accept any); +3. `resolve` and probe it exactly as the original path would — including the + preview trap, which still applies only when the *relocated* target is + NetEase. + +The first success wins and its clip becomes the pick's. Any failure — no hit, +a resolve error, a dead probe, a `SourceAuthError` — falls to the next +catalogue; an auth failure during relocation still goes through `authResult` +so that catalogue closes for the task, but it does **not** end the submit. +All of them failing is not a failure: the submit continues with the original +ref through the unchanged existing path. The finished pick keeps the model's +`title`, `artist` and `announce` — **only the clip changes**. + +**ponytail: the match is containment plus a 20 s window, nothing more.** No +fuzzy distance, no pinyin folding, no romanisation table — a wrong relocation +plays a different song, so the ceiling is deliberately a rule that is cheap to +read and easy to fail closed on. Upgrade path, if real use shows misses worth +paying for: fold both sides through the same converter `folded()` names, and +score candidates rather than taking the first. + +**The knob.** `MURMUR_PLAY_ORDER`, comma-separated, the same +warn-and-default posture as the other `MURMUR_*` music knobs: parsed to a +de-duplicated list of `youtube|bilibili|qqmusic|netease`, with any catalogue +the list omits appended in default order (`MURMUR_PLAY_ORDER=bilibili` means +bilibili first, the rest as they were). An unknown token is rejected — the +whole value is ignored with one warning and the default stands. + +**The dev log** (§3.6 applies: lengths, never words). One line per submit that +attempted a relocation, through the same sink as `music.search` / +`music.resolve` / `music.probe`: + +``` +music.relocate from=netease to=youtube ok +music.relocate from=netease none reason=no-hit # also: dead | resolve-failed | auth +``` + +**What the prompt says** (§3.3, revised): search wherever the song is likeliest +to be **found** — NetEase and QQ Music for Chinese-catalogue depth, Bilibili +and YouTube as well — because where a pick plays from is decided after +`submit_pick`. Choose the best song, not the best source. + --- ## 3. Design @@ -1434,8 +1509,8 @@ sources are not a knob. One paragraph added to the music prompt (`src/prompts/music.ts`), rendered only when a digest is present: the listener's kept music is a strong prior for *style*; pick for the moment; when a kept track fits, it is fine to play -it, but not two in a row; when the listener's taste points at Chinese -catalogue, prefer NetEase or Bilibili search if mounted; say in `announce` +it, but not two in a row; search wherever the song is likeliest to be FOUND, +not where it plays best (§2.13 relocates the pick after submit); say in `announce` where a pick came from only when it is theirs ("one you've kept"). ### 3.4 Boot and refresh @@ -1781,8 +1856,25 @@ the no-argument render in every case (§2.12's scope), and two pack reads across a changed moment render **once** — `TasteReader.renders` proves the memoisation still holds. +### 5.17 Found on NetEase, played from YouTube (unit) — *added 2026-09-20* +A `submit_pick` on a NetEase ref whose title and duration a YouTube hit +matches finishes with the **YouTube** resolve as its clip, keeps the model's +title/artist/announce, and leaves `music.relocate from=netease to=youtube ok` +in the dev log. A YouTube hit more than 20 s off is not taken — the walk falls +to Bilibili and then to the original ref. A submit with no title, and a +segment ref, run no relocation search at all. A `SourceAuthError` from a +relocation search closes that catalogue for the task and the submit still +succeeds. `MURMUR_PLAY_ORDER=bilibili` tries Bilibili first and YouTube +second; an unknown token in it is refused and the default order stands. + ## 6. Resolved decisions +- **Where a song is found and where it plays from are two decisions** + (2026-09-20, user). NetEase's CDN measured ~59 KB/s against a ~1 Mbps FLAC + here, so the model is told to search for the best *song* and code relocates + the pick to the fastest catalogue that has it (§2.13). The alternative — + teaching the prompt to prefer fast sources — trades recall for speed at the + one place the model is actually good, and cannot know today's throughput. - **Taste over transport** (2026-09-06, user). The requirement is recommendation quality; playback stays where it is. - **Spotify read-only, free account** — Premium gates streaming only; the Web diff --git a/src/app.ts b/src/app.ts index a57e7db..a0d5ef6 100644 --- a/src/app.ts +++ b/src/app.ts @@ -373,6 +373,8 @@ function buildMusic( catalogues: taste.catalogues, onAuthFailure: (err) => taste.watch.note(err), probeDurationS: (s, headers, startS) => probePlayableDurationS(s, config.ffmpegCmd, undefined, headers, startS), + // Where a found song plays from (spec 14 §2.13), MURMUR_PLAY_ORDER. + playOrder: config.playOrder, }, }), // Discovery stage timings land in the dev log (issue #76). diff --git a/src/config.ts b/src/config.ts index dbcbe5a..56eb7ee 100644 --- a/src/config.ts +++ b/src/config.ts @@ -13,6 +13,7 @@ import { parseArgs } from 'node:util' import { z } from 'zod' +import { PLAY_ORDER } from './contracts.ts' import { LogEvidenceSchema, resolveLogSource, type LogEvidence } from './support/dev-log.ts' import { dataRoot, @@ -96,6 +97,9 @@ export const ConfigSchema = z.object({ // host's own voice: they run often, in the background, and a pick that misses // is over by the next song. musicModel: z.string().default('claude-sonnet-5'), + // Where a found song is PLAYED from (spec 14 §2.13), best first. Finding and + // playing are two decisions: the model picks the song, this picks the CDN. + playOrder: z.array(z.enum(PLAY_ORDER)).default([...PLAY_ORDER]), // The listener-owned taste half of the pick instruction (spec 03-01 §2.3), // under the one murmur home. Absent = the built-in policy. musicPolicyPath: z.string().default(() => musicPolicyPath()), @@ -258,6 +262,20 @@ function ttsFromEnv(env: NodeJS.ProcessEnv): Partial { } } +// MURMUR_PLAY_ORDER: a comma-separated preference, warn-and-default like every +// other env knob here. Whatever the listener omits keeps its default rank +// behind whatever they named, so naming one catalogue only promotes that one. +function playOrderFromEnv(env: NodeJS.ProcessEnv): Partial { + const raw = env.MURMUR_PLAY_ORDER?.trim() + if (!raw) return {} + const parsed = z.array(z.enum(PLAY_ORDER)).safeParse(raw.split(',').map((t) => t.trim())) + if (!parsed.success) { + console.warn(`warning: ignoring unusable MURMUR_PLAY_ORDER=${JSON.stringify(raw)}`) + return {} + } + return { playOrder: [...new Set([...parsed.data, ...PLAY_ORDER])] } +} + // The MURMUR_RWT_* numbers (spec 13 §2.6): the same warn-and-default posture, // omitted when unset so the schema default stands. function rwtFromEnv(env: NodeJS.ProcessEnv): Partial { @@ -353,6 +371,7 @@ export function parseCli(argv: string[], env: NodeJS.ProcessEnv = process.env): sourcesPath: sourcesConfigPath(env), tasteDir: tasteDir(env), ...rwtFromEnv(env), + ...playOrderFromEnv(env), tuiSocket: tuiSocketPath(env), ...logSource(env), // Having an endpoint IS the reason to speak with it: a voice configured diff --git a/src/contracts.ts b/src/contracts.ts index e1b7eee..058bbeb 100644 --- a/src/contracts.ts +++ b/src/contracts.ts @@ -243,6 +243,12 @@ export type TrackCandidate = { // a place to LOOK, never a statement about the listener's taste. export type Catalogue = 'youtube' | 'bilibili' | 'netease' | 'qqmusic' | 'channels' +// Where a found song is PLAYED from, best first (spec 14 §2.13). `channels` is +// absent on purpose: it is a place to LOOK, and its refs are YouTube or +// Bilibili uploads that read as their own host. +export const PLAY_ORDER = ['youtube', 'bilibili', 'qqmusic', 'netease'] as const +export type PlayCatalogue = (typeof PLAY_ORDER)[number] + // The low-level music source (spec 03-01 §2.2). No start/close: the default // adapter is a binary invoked per call, with nothing to warm or release. // `catalogue` is additive (spec 14 §2.4): absent means youtube, as before. diff --git a/src/music/music-programmer.ts b/src/music/music-programmer.ts index 0eebc6a..6841c20 100644 --- a/src/music/music-programmer.ts +++ b/src/music/music-programmer.ts @@ -126,6 +126,9 @@ export class MusicProgrammer implements TrackSource { const { debug, probe } = this.deps const provider = debug === undefined ? this.deps.provider : timedProvider(this.deps.provider, debug) const wiredProbe = probe !== undefined && debug !== undefined ? timedProbe(probe, debug) : probe + // The relocation line (spec 14 §2.13) rides the same sink as the timings. + const taste = + this.deps.taste === undefined || debug === undefined ? this.deps.taste : { ...this.deps.taste, debug } const t = performance.now() // The situation size rides along because prompt growth is the suspected // hot-slower-than-cold term (spec 04 §3.3 measurement). @@ -139,7 +142,7 @@ export class MusicProgrammer implements TrackSource { // states, and nothing reads its reasoning back. The SDK's default extended // thinking spent ~45 s of a ~100 s pick writing it (issue #164). thinking: 'disabled', - tools: (finish) => musicTools(provider, finish, wiredProbe, this.deps.taste, this.deps.channels, avoid), + tools: (finish) => musicTools(provider, finish, wiredProbe, taste, this.deps.channels, avoid), }) debug?.(`music.pick done ${elapsed(t)} picked=${pick === null ? 'no' : 'yes'}`) return pick diff --git a/src/music/music-tools.ts b/src/music/music-tools.ts index f73a0b7..b9ef327 100644 --- a/src/music/music-tools.ts +++ b/src/music/music-tools.ts @@ -15,9 +15,10 @@ import { tool } from '@anthropic-ai/claude-agent-sdk' import { z } from 'zod' -import type { Catalogue, MusicProvider, TaskTool, TrackCandidate, TrackPick } from '../contracts.ts' +import { PLAY_ORDER, type AudioClip, type Catalogue, type MusicProvider, type PlayCatalogue, type TaskTool, type TrackCandidate, type TrackPick } from '../contracts.ts' import { ANNOUNCE_FIELD_DESCRIPTION } from '../prompts/music.ts' import { previewTrap, SourceAuthError } from './sources/auth.ts' +import { parseSegmentRef } from './music.ts' import { sourceOfRef } from './sources/store.ts' import { SOURCE_NAMES } from './sources/taste.ts' @@ -43,6 +44,12 @@ export type TasteToolOptions = { headers?: Readonly>, startS?: number, ) => Promise + // Where a found song is PLAYED from, best first (spec 14 §2.13). Absent = + // the default order, so a caller that never heard of the knob still relocates. + playOrder?: readonly PlayCatalogue[] + // The dev-log sink MusicProgrammer already feeds; the relocation line joins + // music.search / music.resolve / music.probe there. + debug?: (message: string) => void } function reply(payload: Record) { @@ -79,6 +86,14 @@ function trimmed(value: string | undefined): string | undefined { const CATALOGUES = ['youtube', 'bilibili', 'netease', 'qqmusic', 'channels'] as const +// A relocated hit must be the same song: its title contains the submitted one +// (or the reverse — a catalogue that appends "(Official Audio)" is still it) +// and its length is within this much of what the original candidate claimed. +// ponytail: containment plus a window, no fuzzy distance and no pinyin table — +// a wrong relocation plays a different song, so the rule fails closed. Upgrade +// path in spec 14 §2.13. +const SAME_LENGTH_S = 20 + // The curated-channel pool (spec 14 §2.9), read live: a local match over the // recent uploads of the channels the listener curated. It is offered only // while it holds something, and searching it never touches the network. @@ -131,6 +146,98 @@ export function musicTools( }) } + + // Resolve a ref and prove it will really play: the NetEase preview trap + // (spec 14 §2.6) and then the decoder probe, which the trap's own read + // stands in for when it got one. Shared by the submitted ref and by every + // relocation attempt, so a relocated pick is held to the same bar. + type Opened = + | { ok: true; clip: AudioClip } + | { ok: false; why: 'dead' | 'resolve-failed'; error: string } + | { ok: false; why: 'auth'; err: SourceAuthError } + const openClip = async (ref: string): Promise => { + let clip: AudioClip + try { + clip = await provider.resolve(ref) + } catch (err) { + if (err instanceof SourceAuthError) return { ok: false, why: 'auth', err } + return { ok: false, why: 'resolve-failed', error: err instanceof Error ? err.message : String(err) } + } + // The length the trap read off the stream, null when it never ran or read + // nothing — the playability probe below reads it. + let trapRead: number | null = null + if (taste?.probeDurationS !== undefined && sourceOfRef(ref) === 'netease') { + trapRead = await taste.probeDurationS(clip.source, clip.headers, clip.segment?.startS) + // A segment clip is meant to be its chapter's length; a whole track is + // meant to be the length its candidate claimed. + const expected = clip.segment === undefined ? (stated.get(ref) ?? 0) : clip.segment.endS - clip.segment.startS + if (previewTrap(expected, trapRead)) { + const err = new SourceAuthError('netease', 'login-required', `preview clip of ${String(trapRead)}s`) + return { ok: false, why: 'auth', err } + } + } + // A resolved stream URL can still 403 in the decoder and never produce a + // frame. Reject it now, during talk, so the announce never claims a track + // that turns out silent. + // Unless the trap above just opened this very stream and read a real + // length off it — that IS the proof, and opening it twice costs another + // 13-15 s against a slow NetEase CDN, where the probe's own 15 s ceiling + // then calls a live stream dead and the whole pick starts over (issue + // #164). A trap that read nothing proves nothing, so the probe still runs. + if (trapRead === null && probe !== undefined && !(await probe(clip.source, clip.headers, clip.segment?.startS))) { + return { ok: false, why: 'dead', error: `${ref} resolved but the stream did not play; pick another` } + } + return { ok: true, clip } + } + + // Where the pick will PLAY from (spec 14 §2.13). The model chose the song; + // this chooses the CDN, model-free, and every failure simply falls through + // to the ref the model actually submitted. null = nothing better was found, + // or there was nothing better to look for. + const relocate = async (ref: string, title: string, artist: string | undefined): Promise => { + // A chapter of one specific upload has no equivalent anywhere else. + if (parseSegmentRef(ref).segment !== undefined) return null + const order = taste?.playOrder ?? PLAY_ORDER + // An unknown host is a YouTube ref in everything but spelling — that is + // where a bare search sends the model, and where `channels` uploads live. + const own: PlayCatalogue = sourceOfRef(ref) ?? 'youtube' + const rank = order.indexOf(own) + const better = (rank === -1 ? order : order.slice(0, rank)).filter((c) => open().includes(c)) + if (better.length === 0) return null + + const wanted = folded(title) + const length = stated.get(ref) + let reason = 'no-hit' + for (const catalogue of better) { + let hits: TrackCandidate[] + try { + hits = await provider.search(`${artist ?? ''} ${title}`.trim(), 5, catalogue) + } catch (err) { + // A lost login here closes that catalogue for the task like any other + // auth failure, but it must never end a submit that was going fine. + if (err instanceof SourceAuthError) authResult(err) + reason = err instanceof SourceAuthError ? 'auth' : 'search-failed' + continue + } + for (const hit of hits) stated.set(hit.ref, hit.durationS) + const match = hits.find((hit) => { + const found = folded(hit.title) + const sameSong = found.includes(wanted) || wanted.includes(found) + return sameSong && (length === undefined || Math.abs(hit.durationS - length) <= SAME_LENGTH_S) + }) + if (match === undefined) continue + const opened = await openClip(match.ref) + if (opened.ok) { + taste?.debug?.(`music.relocate from=${own} to=${catalogue} ok`) + return opened.clip + } + if (opened.why === 'auth') authResult(opened.err) + reason = opened.why + } + taste?.debug?.(`music.relocate from=${own} none reason=${reason}`) + return null + } + const searchMusic = tool( 'search_music', 'Search for candidate tracks by query; returns candidates (ref, title, ' + @@ -192,38 +299,17 @@ export function musicTools( return reply({ ok: false, error: `${label} was played recently; pick a different song` }) } - let clip - try { - clip = await provider.resolve(ref) - } catch (err) { - if (err instanceof SourceAuthError) return authResult(err) - return reply({ ok: false, error: err instanceof Error ? err.message : String(err) }) - } - // The preview trap (spec 14 §2.6): NetEase hands a rights-less request a - // 30 s clip with no error, so the decoded length is checked against the - // length the candidate claimed. - // The length the trap read off the stream, null when it never ran or read - // nothing — the playability probe below reads it. - let trapRead: number | null = null - if (taste?.probeDurationS !== undefined && sourceOfRef(ref) === 'netease') { - trapRead = await taste.probeDurationS(clip.source, clip.headers, clip.segment?.startS) - // A segment clip is meant to be its chapter's length; a whole track is - // meant to be the length its candidate claimed. - const expected = clip.segment === undefined ? (stated.get(ref) ?? 0) : clip.segment.endS - clip.segment.startS - if (previewTrap(expected, trapRead)) { - return authResult(new SourceAuthError('netease', 'login-required', `preview clip of ${String(trapRead)}s`)) + // Found is not where it plays from (spec 14 §2.13): a song the model + // found on a slow catalogue is played from the fastest one that also + // has it. Only a pick that names itself can be looked for elsewhere. + let clip = title === undefined ? null : await relocate(ref, title, artist) + if (clip === null) { + const opened = await openClip(ref) + if (!opened.ok) { + if (opened.why === 'auth') return authResult(opened.err) + return reply({ ok: false, error: opened.error }) } - } - // A resolved stream URL can still 403 in the decoder and never produce a - // frame. Reject it now, during talk, so the announce never claims a track - // that turns out silent. - // Unless the trap above just opened this very stream and read a real - // length off it — that IS the proof, and opening it twice costs another - // 13-15 s against a slow NetEase CDN, where the probe's own 15 s ceiling - // then calls a live stream dead and the whole pick starts over (issue - // #164). A trap that read nothing proves nothing, so the probe still runs. - if (trapRead === null && probe !== undefined && !(await probe(clip.source, clip.headers, clip.segment?.startS))) { - return reply({ ok: false, error: `${ref} resolved but the stream did not play; pick another` }) + clip = opened.clip } const announce = trimmed(args.announce) diff --git a/src/prompts/music.ts b/src/prompts/music.ts index 39e3578..0f54548 100644 --- a/src/prompts/music.ts +++ b/src/prompts/music.ts @@ -91,9 +91,10 @@ export const MUSIC_POLICY_HEADER = 'Policy:' export const TASTE_GUIDANCE = `With taste in hand: the block "What the listener keeps" below is what they actually keep on their own platforms — a strong prior for STYLE, not a list to replay. Pick for the moment. When a kept track genuinely fits, playing it is -fine, but not two in a row. When their taste points at a Chinese -catalogue, prefer a NetEase or Bilibili search where search_music lists it as -available. In the announce, say where a pick came from only when it is theirs +fine, but not two in a row. Search wherever the song is likeliest to be FOUND — NetEase +and QQ Music for Chinese-catalogue depth, Bilibili and YouTube as well. +Where a pick PLAYS from is decided after submit_pick, so choose the best song, +not the best source. In the announce, say where a pick came from only when it is theirs ("one you've kept"), never otherwise.` // What the `channels` catalogue IS (spec 14 §2.9), rendered only while the diff --git a/test/config.test.ts b/test/config.test.ts index 33a538c..4507b5c 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -228,6 +228,22 @@ describe('music discovery config', () => { it('defaults musicPolicyPath under the (relocatable) home', () => { expect(parseCli([], { MURMUR_HOME: '/tmp/mh' }).config.musicPolicyPath).toBe('/tmp/mh/music-policy.md') }) + + it('reads the play order from env, dedupes it, and ranks the rest behind (spec 14 §2.13)', () => { + expect(parseCli([], NO_ENV).config.playOrder).toEqual(['youtube', 'bilibili', 'qqmusic', 'netease']) + expect(parseCli([], isolated({ MURMUR_PLAY_ORDER: 'bilibili' })).config.playOrder).toEqual([ + 'bilibili', 'youtube', 'qqmusic', 'netease', + ]) + expect(parseCli([], isolated({ MURMUR_PLAY_ORDER: ' netease , youtube ,netease' })).config.playOrder).toEqual([ + 'netease', 'youtube', 'bilibili', 'qqmusic', + ]) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + expect(parseCli([], isolated({ MURMUR_PLAY_ORDER: 'youtube,spotify' })).config.playOrder).toEqual([ + 'youtube', 'bilibili', 'qqmusic', 'netease', + ]) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('MURMUR_PLAY_ORDER')) + warn.mockRestore() + }) }) // spec 05 §2.3: an installed murmur logs by default, under the one home; only diff --git a/test/fakes.ts b/test/fakes.ts index 3a84295..4643ae4 100644 --- a/test/fakes.ts +++ b/test/fakes.ts @@ -84,9 +84,15 @@ export class FakeMusicProvider implements MusicProvider { // The headers the resolved stream needs (spec 03-01 §2.2), when it needs any. headers: Record | null = null + // Per-catalogue answers, for the tests where the catalogues must differ + // (spec 14 §2.13's relocation). An Error is thrown instead of returned. + byCatalogue: Partial> = {} + async search(query: string, limit?: number, catalogue?: Catalogue): Promise { this.searches.push({ query, limit, catalogue }) - return this.candidates + const scripted = this.byCatalogue[catalogue ?? 'youtube'] + if (scripted instanceof Error) throw scripted + return scripted ?? this.candidates } async resolve(ref: string): Promise { diff --git a/test/music-relocate.test.ts b/test/music-relocate.test.ts new file mode 100644 index 0000000..5bf5cd4 --- /dev/null +++ b/test/music-relocate.test.ts @@ -0,0 +1,141 @@ +// Play-source preference (spec 14 §2.13, acceptance §5.17): a song FOUND on +// any mounted catalogue is PLAYED from the catalogue highest in the play +// order. The relocation is code's, never the model's — these tests drive +// submit_pick directly and read the clip it finishes with. +import { describe, expect, it } from 'vitest' + +import type { Catalogue, PlayCatalogue, TrackCandidate, TrackPick } from '../src/contracts.ts' +import { musicTools } from '../src/music/music-tools.ts' +import { SourceAuthError } from '../src/music/sources/auth.ts' +import { callTool, FakeMusicProvider } from './fakes.ts' + +const NETEASE_REF = 'https://music.163.com/#/song?id=5' + +function candidate(ref: string, over: Partial = {}): TrackCandidate { + return { ref, title: 'Kong Kong', uploader: 'Chen Li', durationS: 240, extra: {}, ...over } +} + +function build(opts: { + mounted?: Catalogue[] + playOrder?: PlayCatalogue[] + byCatalogue?: Partial> + broken?: string[] + dead?: string[] +} = {}) { + const provider = new FakeMusicProvider() + provider.candidates = [candidate(NETEASE_REF)] + provider.byCatalogue = { netease: [candidate(NETEASE_REF)], youtube: [], bilibili: [], qqmusic: [], ...opts.byCatalogue } + for (const ref of opts.broken ?? []) provider.broken.add(ref) + const dead = new Set(opts.dead ?? []) + const picks: TrackPick[] = [] + const auth: SourceAuthError[] = [] + const log: string[] = [] + const tools = musicTools( + provider, + (pick) => picks.push(pick), + async (source) => !dead.has(source), + { + catalogues: () => opts.mounted ?? ['netease', 'bilibili'], + onAuthFailure: (err) => auth.push(err), + ...(opts.playOrder !== undefined && { playOrder: opts.playOrder }), + debug: (m) => log.push(m), + }, + ) + return { provider, tools, picks, auth, log } +} + +const submit = (tools: ReturnType['tools'], over: Record = {}) => + callTool(tools, 'submit_pick', { ref: NETEASE_REF, why: 'w', title: 'Kong Kong', artist: 'Chen Li', ...over }) + +// The searches a relocation ran, in order — the netease search that seeded +// the stated length is not one of them. +const relocations = (provider: FakeMusicProvider) => provider.searches.filter((s) => s.catalogue !== 'netease') + +describe('play-source preference (spec 14 §2.13)', () => { + it('plays a NetEase pick from YouTube when YouTube has the same song', async () => { + const yt = 'https://www.youtube.com/watch?v=abc' + const { tools, picks, provider, log } = build({ byCatalogue: { youtube: [candidate(yt, { durationS: 238 })] } }) + await callTool(tools, 'search_music', { query: 'kong kong', catalogue: 'netease' }) + const result = await submit(tools) + + expect(result.ok).toBe(true) + expect(picks[0]?.clip.source).toBe(`https://stream/${yt}`) + // The model's own words survive the relocation; only the clip moved. + expect(picks[0]).toMatchObject({ title: 'Kong Kong', artist: 'Chen Li' }) + expect(relocations(provider)[0]).toEqual({ query: 'Chen Li Kong Kong', limit: 5, catalogue: 'youtube' }) + expect(log).toContain('music.relocate from=netease to=youtube ok') + }) + + it('refuses a hit more than 20 s off, falls to bilibili, then keeps the original', async () => { + const { tools, picks, provider, log } = build({ + byCatalogue: { + youtube: [candidate('https://www.youtube.com/watch?v=abc', { durationS: 300 })], + bilibili: [candidate('https://www.bilibili.com/video/BV1', { title: 'something else' })], + }, + }) + await callTool(tools, 'search_music', { query: 'kong kong', catalogue: 'netease' }) + await submit(tools) + + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + expect(relocations(provider).map((s) => s.catalogue)).toEqual(['youtube', 'bilibili']) + expect(log).toContain('music.relocate from=netease none reason=no-hit') + }) + + it('does not relocate a submit that carries no title', async () => { + const { tools, picks, provider, log } = build() + await submit(tools, { title: undefined, artist: undefined }) + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + expect(relocations(provider)).toEqual([]) + expect(log.filter((l) => l.startsWith('music.relocate'))).toEqual([]) + }) + + it('does not relocate a segment ref', async () => { + const ref = 'https://www.bilibili.com/video/BV1#t=612,868' + const { tools, picks, provider } = build({ mounted: ['bilibili'] }) + await callTool(tools, 'submit_pick', { ref, why: 'w', title: 'Kong Kong' }) + expect(picks[0]?.clip.segment).toEqual({ startS: 612, endS: 868 }) + expect(relocations(provider)).toEqual([]) + }) + + it('does not relocate a ref whose catalogue is already top-ranked', async () => { + const { tools, provider } = build({ mounted: ['netease'] }) + await callTool(tools, 'submit_pick', { ref: 'https://www.youtube.com/watch?v=abc', why: 'w', title: 'Kong Kong' }) + expect(relocations(provider)).toEqual([]) + }) + + it('closes a catalogue whose relocation search hits auth, and still finishes the submit', async () => { + const { tools, picks, auth, provider, log } = build({ + byCatalogue: { youtube: new SourceAuthError('youtube', 'login-required', 'expired') }, + }) + const result = await submit(tools) + + expect(result.ok).toBe(true) + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + expect(auth.map((e) => e.source)).toEqual(['youtube']) + expect(relocations(provider).map((s) => s.catalogue)).toEqual(['youtube', 'bilibili']) + expect(log).toContain('music.relocate from=netease none reason=auth') + // The closed catalogue is gone from what search_music will still take. + expect(await callTool(tools, 'search_music', { query: 'q', catalogue: 'youtube' })).toMatchObject({ ok: false, reason: 'unavailable' }) + }) + + it('falls past a hit that resolves but does not play', async () => { + const yt = 'https://www.youtube.com/watch?v=abc' + const bili = 'https://www.bilibili.com/video/BV1' + const { tools, picks, log } = build({ + byCatalogue: { youtube: [candidate(yt)], bilibili: [candidate(bili)] }, + dead: [`https://stream/${yt}`], + }) + await submit(tools) + expect(picks[0]?.clip.source).toBe(`https://stream/${bili}`) + expect(log).toContain('music.relocate from=netease to=bilibili ok') + }) + + it('walks the configured play order', async () => { + const { tools, provider, log } = build({ + playOrder: ['bilibili', 'youtube', 'qqmusic', 'netease'], + }) + await submit(tools) + expect(relocations(provider).map((s) => s.catalogue)).toEqual(['bilibili', 'youtube']) + expect(log).toContain('music.relocate from=netease none reason=no-hit') + }) +}) diff --git a/test/music-taste.test.ts b/test/music-taste.test.ts index e3a4a7a..7f09576 100644 --- a/test/music-taste.test.ts +++ b/test/music-taste.test.ts @@ -461,7 +461,9 @@ describe('the digest in the prompts (spec 14 §2.3/§3.3)', () => { expect(buildFindMusicInstruction(undefined, { taste: false })).not.toContain(TASTE_GUIDANCE) expect(buildFindMusicInstruction()).not.toContain(TASTE_GUIDANCE) expect(TASTE_GUIDANCE).toMatch(/not two in a row/) - expect(TASTE_GUIDANCE).toMatch(/NetEase or Bilibili/) + // Found, not fast: where a pick plays from is §2.13's, not the model's. + expect(TASTE_GUIDANCE).toMatch(/likeliest to be FOUND/) + expect(TASTE_GUIDANCE).toMatch(/best song,\nnot the best source/) expect(TASTE_GUIDANCE).toMatch(/one you've kept/) }) From 22847911cbb1ae526e5686d3ca6671fb7e47ffab Mon Sep 17 00:00:00 2001 From: wine-fall <62830944+wine-fall@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:33:51 +0800 Subject: [PATCH 2/3] feat(sources): the play order is a card in /sources, not an env var [spec 14] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Where a song plays from is the listener's preference, not a deployment setting. `playOrder` becomes a settings-layer knob beside musicEnabled and musicEveryN — persisted in settings.json, layered file < env < flag, and read through a getter at submit time, so what the card writes lands on the next pick instead of the next launch. MURMUR_PLAY_ORDER stays as the env override. The /sources menu gains a second action row, `( play order )`, always offered because YouTube is always there to order against. Pressing it opens a single-select card whose only verb is promotion — a pick moves that catalogue to the front and the same card is drawn again, ranked, until ( done ). Esc abandons the visit and restores the order it opened on; the menu comes back with `ok play order: Bilibili > YouTube > QQ Music > NetEase`, or unchanged. Only mounted catalogues are shown, and the stored list still holds all four, so an unmounted one keeps its place until the day it is connected. Also from the mid-point codex review, each with the regression test that would have caught it: a hit whose words say it is a karaoke, instrumental or cover take is refused, and so is one with the artist nowhere in its title or uploader — relocating onto either plays the wrong audio under the model's own announce; a channels candidate now seeds its stated length, so the 20 s window covers that path too; and the whole relocation runs under a 15 s budget, because an optimisation may not become the thing a pick waits on. Co-Authored-By: Claude Opus 5 --- specs/spec14/14-listening-taste.md | 71 +++++++++++++- src/app.ts | 22 +++-- src/host/ipc.ts | 7 +- src/music/music-tools.ts | 79 ++++++++++++++-- src/music/sources/flow.ts | 116 ++++++++++++++++++++++- test/director.test.ts | 1 + test/ipc-host.test.ts | 1 + test/ipc.test.ts | 3 + test/music-relocate.test.ts | 80 +++++++++++++++- test/settings.test.ts | 1 + test/setup.test.ts | 1 + test/sources-flow.test.ts | 146 ++++++++++++++++++++++++++++- test/steer-tools.test.ts | 1 + test/tui-settings.test.ts | 1 + 14 files changed, 504 insertions(+), 26 deletions(-) diff --git a/specs/spec14/14-listening-taste.md b/specs/spec14/14-listening-taste.md index 7f77724..cb8c0ae 100644 --- a/specs/spec14/14-listening-taste.md +++ b/specs/spec14/14-listening-taste.md @@ -1126,12 +1126,51 @@ read and easy to fail closed on. Upgrade path, if real use shows misses worth paying for: fold both sides through the same converter `folded()` names, and score candidates rather than taking the first. -**The knob.** `MURMUR_PLAY_ORDER`, comma-separated, the same +**The knob.** `playOrder` is a **settings-layer** knob (spec 12), like +`musicEnabled` and `musicEveryN`: it lives in `~/.murmur/settings.json`, it is +set from the /sources card below, and it is read **per submit** through a +getter — never a value captured at boot — so a change lands on the next pick +rather than the next launch. Layering is the settings layering: file < env < +flag. The env override is `MURMUR_PLAY_ORDER`, comma-separated, with the same warn-and-default posture as the other `MURMUR_*` music knobs: parsed to a de-duplicated list of `youtube|bilibili|qqmusic|netease`, with any catalogue the list omits appended in default order (`MURMUR_PLAY_ORDER=bilibili` means bilibili first, the rest as they were). An unknown token is rejected — the -whole value is ignored with one warning and the default stands. +whole value is ignored with one warning and the default stands. **The stored +list always holds all four**, mounted or not, so an unmounted catalogue keeps +its place and resumes it the day it is connected. + + +**The card** (spec 10 §3.2-B, single-select, `multi: false`), reached from the +/sources menu's `( play order )` action row and re-rendered **in place** after +every pick: + +``` +Play from which first? (a pick moves it to the front) +ok play order: Bilibili > YouTube > QQ Music > NetEase <- the previous pick +>> 1) [x] Bilibili - 1st +>> 2) [ ] YouTube - 2nd +>> 3) ( done ) - keep this order +``` + +- One option row per catalogue **currently mounted** (YouTube always; the rest + as `sources.json` says), listed in the current play order and labelled with + its rank, the current first ticked. Unmounted catalogues are not shown. +- **A pick moves that catalogue to the front**, everything else keeping its + relative order; it is persisted at once and the same card is drawn again. +- `( done )`, or Enter with the current first still selected, returns to the + /sources menu, which leads with `ok play order: > > > ` — or + `ok play order unchanged` when nothing moved. +- **Esc abandons the visit**: the order that stood when the card opened is + restored, however many picks were made inside it, and the menu comes back + with `ok play order unchanged`. +- Plain host: numbers or names, the same one-word-fails-the-line rule as the + menu itself. + +ponytail: promotion is the whole vocabulary — no drag, no move-up/move-down, +no rank typing. Three picks put four catalogues in any order, and one key per +visit is the smallest thing that can. Upgrade path, if four ever become +twelve: a second key for demotion. **The dev log** (§3.6 applies: lengths, never words). One line per submit that attempted a relocation, through the same sink as `music.search` / @@ -1174,6 +1213,7 @@ ok connected NetEase — signed in as Chen X · 312 liked ← last submit's re >> 5) [ ] Soda Music - not connected >> 6) [ ] QQ Music - not connected >> 7) ( refresh now ) - re-read every connected account now ← only once something is mounted +>> 8) ( play order ) - which catalogue a found song plays from first ``` - **Ticked = mounted**, an expired login included: unticking it is how it @@ -1197,6 +1237,15 @@ ok connected NetEase — signed in as Chen X · 312 liked ← last submit's re box for the same reason; typing `refresh` still names it, and so does `refresh now`, the way it is drawn — a listener types the row they can see, and one word the flow cannot place fails the whole line (codex review). +- **`( play order )` is the second ACTION row** (§2.13), placed after the + refresh row and **always present** — YouTube is always there to order + against, so there is always something to answer. Pressing it opens the + play-order card (§2.13), and pressing it is *all* that line does: an action + row is a button, not part of the selection, so nothing else typed alongside + it is read as a tick and nothing is mounted or unmounted by that visit. + Typing `play order` names it on the plain host, the way the row reads. When + the card closes the menu comes back with its result leading, under the same + results-land-in-the-next-card rule every other row follows. - **The TUI's multi card closes on an `( apply )` row it synthesizes itself** (10 §3.2-D) — not on the wire, not this flow's business: the card offered nothing that looked like a submit. Its note says what applying would do @@ -1867,8 +1916,26 @@ relocation search closes that catalogue for the task and the submit still succeeds. `MURMUR_PLAY_ORDER=bilibili` tries Bilibili first and YouTube second; an unknown token in it is refused and the default order stands. +### 5.18 The play order is set from /sources (unit, scripted host) — *added 2026-09-20* +A scripted /sources session: the menu carries `( play order )` as an action +row after `( refresh now )`; pressing it opens the card in the current order +with only the mounted catalogues shown and ranked, the first ticked; picking +Bilibili re-renders the **same** card with Bilibili 1st and +`ok play order: Bilibili > YouTube` leading it; `( done )` returns to the menu +with `ok play order: Bilibili > YouTube > QQ Music > NetEase`; `settings.json` +holds that list; and a `submit_pick` through tools built **before** the card +ran relocates by the new order — no restart. Esc after a pick restores the +order the card opened on. An unmounted catalogue is never shown and keeps its +stored position behind the promotion. + ## 6. Resolved decisions +- **The play order is a card, not an env var** (2026-09-20, user). Where a + song plays from is the listener's preference, not a deployment setting, so + it is a settings-layer knob set from `( play order )` in /sources and read + per submit; `MURMUR_PLAY_ORDER` stays as the env override above the file. + The card's verb is promotion — a pick moves that catalogue to the front — + because it needs one key per visit and no ordering vocabulary at all. - **Where a song is found and where it plays from are two decisions** (2026-09-20, user). NetEase's CDN measured ~59 KB/s against a ~1 Mbps FLAC here, so the model is told to search for the best *song* and code relocates diff --git a/src/app.ts b/src/app.ts index a0d5ef6..d8d6f3c 100644 --- a/src/app.ts +++ b/src/app.ts @@ -246,6 +246,7 @@ export function buildSettingsStore( muted: resolved.muted, tuiPet: resolved.tuiPet, rwtEnabled: resolved.rwtEnabled, + playOrder: resolved.playOrder, }, touched: stored, log, @@ -373,8 +374,9 @@ function buildMusic( catalogues: taste.catalogues, onAuthFailure: (err) => taste.watch.note(err), probeDurationS: (s, headers, startS) => probePlayableDurationS(s, config.ffmpegCmd, undefined, headers, startS), - // Where a found song plays from (spec 14 §2.13), MURMUR_PLAY_ORDER. - playOrder: config.playOrder, + // Where a found song plays from (spec 14 §2.13). Read per submit, so + // the /sources card lands on the next pick without a restart. + playOrder: () => settings.current().playOrder, }, }), // Discovery stage timings land in the dev log (issue #76). @@ -733,6 +735,11 @@ export async function runApp(config: Config, maxSegments?: number): Promise host.info(m)) // The listener's taste (spec 14), on a real run only. Built BEFORE the first // run: its sources card (§3.9) runs the same /sources conversation the // Director later parks on, so the one closure serves both. @@ -750,6 +757,12 @@ export async function runApp(config: Config, maxSegments?: number): Promise buildSource(id, entry, taste.build), forgetCookies: () => taste.build.jars.drop(), + // Where a found song plays from (spec 14 §2.13): the card writes + // through the same authority the pick reads at submit time. + playOrder: { + read: () => settings.current().playOrder, + write: (order) => void settings.set({ playOrder: [...order] }), + }, openUrl: openInChrome, }) let personaPath = resolvePersonaPath(config, persistent) @@ -781,11 +794,6 @@ export async function runApp(config: Config, maxSegments?: number): Promise voiceAuthDown.current }) - // The live settings authority (spec 12 §2.4), seeded from the merged config: - // everything below reads it instead of captured scalars. Built BEFORE the - // setup conversation, which turns its language knob (§3.9); the voice knobs - // that conversation can change are not settings, so nothing here waits on it. - const settings = buildSettingsStore(config, (m) => host.info(m)) let setupMusicOk = false if (claude !== null && !quit.requested) { const outcome = await runSetup({ diff --git a/src/host/ipc.ts b/src/host/ipc.ts index 8aa3d1f..c787269 100644 --- a/src/host/ipc.ts +++ b/src/host/ipc.ts @@ -89,6 +89,10 @@ export const SettingsValuesSchema = z.object({ tuiPet: z.boolean(), // Whether the host is offered real-world material at all (spec 13 §2.6). rwtEnabled: z.boolean(), + // Where a found song is PLAYED from, best first (spec 14 §2.13). Set from + // the /sources play-order card; the ids are spelled out here for the same + // reason SourceLine's are — this module ships with the front-end. + playOrder: z.array(z.enum(['youtube', 'bilibili', 'qqmusic', 'netease'])), // The one OPTIONAL knob (spec 12 §3.9). Absent means the listener never said, // and the persona decides; set is an override applied as a directive on top // of the persona, never an edit to persona.md. Free text — a language name as @@ -98,7 +102,7 @@ export const SettingsValuesSchema = z.object({ export type Settings = z.infer -// A mutation (spec 12 §2.4): a partial over the same nine knobs. +// A mutation (spec 12 §2.4): a partial over the same knobs. export const SettingsPatchSchema = z.object({ anchorsEnabled: z.boolean().optional(), musicEnabled: z.boolean().optional(), @@ -109,6 +113,7 @@ export const SettingsPatchSchema = z.object({ muted: z.boolean().optional(), tuiPet: z.boolean().optional(), rwtEnabled: z.boolean().optional(), + playOrder: z.array(z.enum(['youtube', 'bilibili', 'qqmusic', 'netease'])).optional(), // Empty string is legal HERE and only here: it is how the listener clears the // override and hands the language back to the persona (spec 12 §3.9). language: z.union([LanguageSchema, z.literal('')]).optional(), diff --git a/src/music/music-tools.ts b/src/music/music-tools.ts index b9ef327..cd2a311 100644 --- a/src/music/music-tools.ts +++ b/src/music/music-tools.ts @@ -44,9 +44,11 @@ export type TasteToolOptions = { headers?: Readonly>, startS?: number, ) => Promise - // Where a found song is PLAYED from, best first (spec 14 §2.13). Absent = - // the default order, so a caller that never heard of the knob still relocates. - playOrder?: readonly PlayCatalogue[] + // Where a found song is PLAYED from, best first (spec 14 §2.13). Read per + // submit like `catalogues`, so the /sources card lands on the next pick + // rather than the next boot. Absent = the default order, so a caller that + // never heard of the knob still relocates. + playOrder?: () => readonly PlayCatalogue[] // The dev-log sink MusicProgrammer already feeds; the relocation line joins // music.search / music.resolve / music.probe there. debug?: (message: string) => void @@ -94,6 +96,50 @@ const CATALOGUES = ['youtube', 'bilibili', 'netease', 'qqmusic', 'channels'] as // path in spec 14 §2.13. const SAME_LENGTH_S = 20 +// How long the whole relocation may take before the submit gives up on it and +// plays what the model picked. Measured: a real YouTube search + resolve + +// probe is ~5 s, so this leaves room for one miss and still lands well inside +// the talk that covers a pick. +const RELOCATE_BUDGET_MS = 15_000 + +// ponytail: one shared timer, raced against each step — not a per-call +// AbortSignal. yt-dlp is spawned by the provider and neither it nor ffmpeg +// takes a signal from here, so the only thing that can be cut short is the +// waiting; the abandoned spawn ends on its own ceiling. +function deadlineIn(ms: number): { passed: () => boolean; race: (work: Promise) => Promise } { + const until = performance.now() + ms + return { + passed: () => performance.now() >= until, + race: (work: Promise): Promise => + Promise.race([ + work, + new Promise((_, reject) => { + const timer = setTimeout(() => reject(new Error('relocation budget spent')), Math.max(0, until - performance.now())) + void work.finally(() => clearTimeout(timer)).catch(() => {}) + }), + ]), + } +} + +// A different RECORDING of the same title is a different song to a listener: +// a karaoke backing track is 240 s of the right length with the right name. +// A hit whose words say it is one of these, where the submitted title did not, +// is refused — and refusing costs nothing but the speed-up. +const OTHER_RECORDING = + // The Chinese markers are escaped because committed source is English only + // (AGENTS.md): \u4f34\u594f backing track, \u7ffb\u5531 cover, + // \u7eaf\u97f3\u4e50 instrumental, \u6296\u97f3\u7248 / dj\u7248 edits. + /karaoke|instrumental|cover|remix|nightcore|sped up|slowed|8d audio|\u4f34\u594f|\u7ffb\u5531|\u7eaf\u97f3\u4e50|\u6296\u97f3\u7248|dj\u7248/ + +// Whoever the model said made it has to show up somewhere in the hit — its +// title or its uploader — before the pick is moved onto it. A catalogue that +// spells the artist differently simply keeps the pick where it was. +function sameArtist(hit: TrackCandidate, artist: string | undefined): boolean { + if (artist === undefined) return true + const wanted = folded(artist) + return folded(hit.title).includes(wanted) || folded(hit.uploader).includes(wanted) +} + // The curated-channel pool (spec 14 §2.9), read live: a local match over the // recent uploads of the channels the listener curated. It is offered only // while it holds something, and searching it never touches the network. @@ -197,7 +243,7 @@ export function musicTools( const relocate = async (ref: string, title: string, artist: string | undefined): Promise => { // A chapter of one specific upload has no equivalent anywhere else. if (parseSegmentRef(ref).segment !== undefined) return null - const order = taste?.playOrder ?? PLAY_ORDER + const order = taste?.playOrder?.() ?? PLAY_ORDER // An unknown host is a YouTube ref in everything but spelling — that is // where a bare search sends the model, and where `channels` uploads live. const own: PlayCatalogue = sourceOfRef(ref) ?? 'youtube' @@ -208,25 +254,36 @@ export function musicTools( const wanted = folded(title) const length = stated.get(ref) let reason = 'no-hit' + // Relocation is an optimisation, and an optimisation may not become the + // thing the pick waits on: a single yt-dlp call can sit for 90 s, and the + // Director is filling that silence with talk. Past this the original ref + // resolves as it always would (codex review). + const deadline = deadlineIn(RELOCATE_BUDGET_MS) for (const catalogue of better) { + if (deadline.passed()) { + reason = 'timed-out' + break + } let hits: TrackCandidate[] try { - hits = await provider.search(`${artist ?? ''} ${title}`.trim(), 5, catalogue) + hits = await deadline.race(provider.search(`${artist ?? ''} ${title}`.trim(), 5, catalogue)) } catch (err) { // A lost login here closes that catalogue for the task like any other // auth failure, but it must never end a submit that was going fine. if (err instanceof SourceAuthError) authResult(err) - reason = err instanceof SourceAuthError ? 'auth' : 'search-failed' + reason = err instanceof SourceAuthError ? 'auth' : deadline.passed() ? 'timed-out' : 'search-failed' continue } for (const hit of hits) stated.set(hit.ref, hit.durationS) const match = hits.find((hit) => { const found = folded(hit.title) const sameSong = found.includes(wanted) || wanted.includes(found) - return sameSong && (length === undefined || Math.abs(hit.durationS - length) <= SAME_LENGTH_S) + if (!sameSong || !sameArtist(hit, artist)) return false + if (OTHER_RECORDING.test(found) && !OTHER_RECORDING.test(wanted)) return false + return length === undefined || Math.abs(hit.durationS - length) <= SAME_LENGTH_S }) if (match === undefined) continue - const opened = await openClip(match.ref) + const opened = await deadline.race(openClip(match.ref)).catch((): Opened => ({ ok: false, why: 'dead', error: 'timed out' })) if (opened.ok) { taste?.debug?.(`music.relocate from=${own} to=${catalogue} ok`) return opened.clip @@ -261,7 +318,11 @@ export function musicTools( if (closed.has(catalogue ?? 'youtube')) return reply({ ok: false, reason: 'unavailable', mounted: open() }) // The curated channels are already on disk: matched here, so a search of // them costs nothing and cannot fail (spec 14 §2.9). - if (catalogue === 'channels') return reply({ candidates: channels?.search(args.query, args.limit) ?? [] }) + if (catalogue === 'channels') { + const found = channels?.search(args.query, args.limit) ?? [] + for (const c of found) stated.set(c.ref, c.durationS) + return reply({ candidates: found }) + } try { const candidates = await provider.search(args.query, args.limit, catalogue) for (const c of candidates) stated.set(c.ref, c.durationS) diff --git a/src/music/sources/flow.ts b/src/music/sources/flow.ts index 5181f32..7c20e0a 100644 --- a/src/music/sources/flow.ts +++ b/src/music/sources/flow.ts @@ -5,6 +5,7 @@ // parking: the loop waits inside it while the music plays on. It is the // single writer of sources.json for its whole duration (store.busy). +import type { PlayCatalogue } from '../../contracts.ts' import type { Host, InfoTone } from '../../host/host.ts' import { ask } from '../../host/host.ts' import type { AskOption } from '../../host/ipc.ts' @@ -77,6 +78,9 @@ export type SourcesFlowDeps = { refresher: TasteRefresher watch: SourceAuthWatch mounts: SourceMounts + // Where a found song is PLAYED from (spec 14 §2.13), read and written live + // through the settings store. Absent = the row is not offered at all. + playOrder?: { read: () => readonly PlayCatalogue[]; write: (order: readonly PlayCatalogue[]) => void } // The live adapter for a freshly mounted entry, for the first snapshot. build: (id: SourceId, entry: SourceEntry[SourceId]) => TasteSource | null // Drop whatever was cached from the browser's cookie store: a mount that @@ -121,9 +125,12 @@ const NAMES: Record = { // The row is drawn as a button — `( refresh now )` — so that is what gets // typed, and one word the flow cannot place fails the whole line. now: 'refresh', + // `( play order )`, read the same way: both of its words name the row. + play: 'playOrder', + order: 'playOrder', } -type MenuKey = SourceId | 'refresh' +type MenuKey = SourceId | 'refresh' | 'playOrder' type MenuRow = AskOption & { key: MenuKey; note: string; checked: boolean } const QUESTION = 'which accounts should I read? Enter with nothing changed leaves' @@ -131,6 +138,9 @@ const QUESTION = 'which accounts should I read? Enter with nothing changed leave // now, not something you are connected to, so the row carries `action` and // is drawn as a button — on the card and in the numbered rows alike. const REFRESH_ROW: MenuRow = { key: 'refresh', label: 'refresh now', note: 're-read every connected account now', checked: false, action: true } +// The other ACTION row (spec 14 §2.13): where a song plays FROM, once it has +// been found. Always offered — YouTube is always there to order against. +const PLAY_ORDER_ROW: MenuRow = { key: 'playOrder', label: 'play order', note: 'which catalogue a found song plays from first', checked: false, action: true } function ago(iso: string | undefined, now: Date): string { if (iso === undefined) return 'never read' @@ -167,7 +177,7 @@ function counts(store: SourcesStore, id: SourceId): string { // when it is mounted (an expired login included — unticking it is how it // is forgotten without signing in), its note the state; a refresh row once // anything is mounted. -function menuRows(store: SourcesStore, now: Date): MenuRow[] { +function menuRows(store: SourcesStore, now: Date, playOrder: SourcesFlowDeps['playOrder']): MenuRow[] { const file = store.read() const rows: MenuRow[] = SOURCE_IDS.map((id) => { const entry = file[id] @@ -176,7 +186,8 @@ function menuRows(store: SourcesStore, now: Date): MenuRow[] { if (entry.status === 'expired') return { key: id, label, note: 'expired — untick to forget it, tick refresh to sign in again', checked: true } return { key: id, label, note: `${counts(store, id)} · ${ago(entry.lastRefresh, now)}`, checked: true } }) - return store.mounted().length > 0 ? [...rows, REFRESH_ROW] : rows + const actions = [...(store.mounted().length > 0 ? [REFRESH_ROW] : []), ...(playOrder === undefined ? [] : [PLAY_ORDER_ROW])] + return [...rows, ...actions] } // The card text: the question, the previous submit's results as ready/gap @@ -430,6 +441,92 @@ function obstacleLine(reason: CookieFailure, detail: string, platform: NodeJS.Pl : "Chrome is here, but I am not allowed to read its cookie store — check this terminal's permissions, then /sources again." } + +// --- the play-order card (spec 14 §2.13 / §3.1) --------------------------- // + +// A single-select card whose semantics are "a pick moves it to the front": +// ordering by repeated promotion needs one key per visit and no up/down verbs, +// and three picks put any four-item list in any order. +// ponytail: no drag, no move-up/move-down, no rank typing — promotion is the +// whole vocabulary. Upgrade path, if four catalogues ever become twelve: a +// second key for demotion. +export const PLAY_ORDER_QUESTION = 'Play from which first? (a pick moves it to the front)' +const DONE_ROW: AskOption = { key: 'done', label: 'done', note: 'keep this order', action: true } +const RANKS = ['1st', '2nd', '3rd', '4th'] as const + +const ordinal = (i: number): string => RANKS[i] ?? `${String(i + 1)}th` + +// The stored list always holds all four; the card shows only what is mounted, +// so an unmounted catalogue keeps its place without ever being asked about. +function shown(order: readonly PlayCatalogue[], store: SourcesStore): PlayCatalogue[] { + const mounted = new Set(store.mounted()) + return order.filter((c) => c === 'youtube' || mounted.has(c)) +} + +// Promote one catalogue to the front, everything else in the order it had. +export function promote(order: readonly PlayCatalogue[], first: PlayCatalogue): PlayCatalogue[] { + return [first, ...order.filter((c) => c !== first)] +} + +// How the order reads once it is set: the result row the menu comes back with. +export const orderLine = (order: readonly PlayCatalogue[]): string => + `ok play order: ${order.map((c) => SOURCE_NAMES[c]).join(' > ')}` + +function playOrderRows(order: readonly PlayCatalogue[], store: SourcesStore): (AskOption & { key: string })[] { + const rows = shown(order, store).map((c, i) => ({ + key: c, + label: SOURCE_NAMES[c], + note: ordinal(i), + checked: i === 0, + })) + return [...rows, DONE_ROW] +} + +function playOrderText(rows: readonly AskOption[], result: string | undefined): string { + const numbered = rows.map((row, i) => `>> ${String(i + 1)}) ${row.action === true ? `( ${row.label} )` : `[${row.checked === true ? 'x' : ' '}] ${row.label}`} - ${row.note ?? ''}`) + return [PLAY_ORDER_QUESTION, ...(result === undefined ? [] : [result]), ...numbered].join('\n') +} + +// Re-rendered in place after every pick until `( done )` or Enter on the +// current first. Esc abandons the visit and restores the order it opened on. +// Returns the result row the /sources menu leads with. +async function runPlayOrder( + deps: SourcesFlowDeps & { playOrder: NonNullable }, + read: () => Promise, + stopped: () => boolean, +): Promise { + const { host, store, playOrder } = deps + const opened = [...playOrder.read()] + let order = [...opened] + let result: string | undefined + for (;;) { + const rows = playOrderRows(order, store) + ask(host, playOrderText(rows, result), 'question', { options: rows, multi: false }) + const line = (await read()).trim().toLowerCase() + if (stopped()) { + // Esc abandons the visit: the order that stood when the card opened is + // what the radio goes back to, however many picks were made inside it. + playOrder.write(opened) + return 'ok play order unchanged' + } + // Enter with the current first still selected, or the done button. + if (line === '' || line === 'done') break + const picked = rows.find( + (row, i) => row.key === line || row.label.toLowerCase() === line || String(i + 1) === line, + ) + if (picked === undefined) { + result = `-- I didn't catch "${line}" — numbers or names from the list` + continue + } + if (picked.key === 'done') break + order = promote(order, picked.key as PlayCatalogue) + playOrder.write(order) + result = orderLine(order) + } + const moved = order.some((c, i) => opened[i] !== c) + return moved ? orderLine(order) : 'ok play order unchanged' +} + export async function runSources(deps: SourcesFlowDeps): Promise { const { host, store, quit } = deps const now = deps.now ?? (() => new Date()) @@ -452,7 +549,7 @@ export async function runSources(deps: SourcesFlowDeps): Promise { try { let results: string[] = [] while (!quit.requested) { - const rows = menuRows(store, now()) + const rows = menuRows(store, now(), deps.playOrder) ask(host, menuText(rows, results), 'question', { options: rows, multi: true }) results = [] cancelled = false @@ -467,6 +564,17 @@ export async function runSources(deps: SourcesFlowDeps): Promise { results.push(`-- ${picked}`) continue } + // An action row is a button, not part of the selection: pressing it does + // its one thing and brings the menu back, so nothing else on the line + // can be read as "untick everything else". + if (picked.has('playOrder') && deps.playOrder !== undefined) { + cancelled = false + const withOrder = { ...deps, playOrder: deps.playOrder } + results.push(await runPlayOrder(withOrder, read, () => cancelled || gone || quit.requested)) + if (gone || quit.requested) return + cancelled = false + continue + } // The diff against what stands: unticked-and-mounted goes, ticked-and- // not is signed in, refresh re-reads (an expired login is signed in // again first — a re-read cannot renew it). Nothing changed = done. diff --git a/test/director.test.ts b/test/director.test.ts index edf73ac..da50c3a 100644 --- a/test/director.test.ts +++ b/test/director.test.ts @@ -916,6 +916,7 @@ describe('Director — a language change invalidates the talk look-ahead (spec 1 muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], }, touched: {}, }) diff --git a/test/ipc-host.test.ts b/test/ipc-host.test.ts index dab9856..0aed08a 100644 --- a/test/ipc-host.test.ts +++ b/test/ipc-host.test.ts @@ -734,6 +734,7 @@ describe('IpcHost (spec 10 §2.1/§2.3)', () => { muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], } function wire(applyOk = true): SettingsPatch[] { diff --git a/test/ipc.test.ts b/test/ipc.test.ts index 6794674..8e67dca 100644 --- a/test/ipc.test.ts +++ b/test/ipc.test.ts @@ -76,6 +76,7 @@ const ENGINE_MESSAGES: EngineMessage[] = [ muted: true, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], }, home: '/home/someone/.murmur', voiceConfigured: true, @@ -94,6 +95,7 @@ const ENGINE_MESSAGES: EngineMessage[] = [ muted: false, tuiPet: false, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], }, home: '/tmp/m', voiceConfigured: false, @@ -116,6 +118,7 @@ const ENGINE_MESSAGES: EngineMessage[] = [ muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], }, home: '/home/someone/.murmur', voiceConfigured: true, diff --git a/test/music-relocate.test.ts b/test/music-relocate.test.ts index 5bf5cd4..4eb433c 100644 --- a/test/music-relocate.test.ts +++ b/test/music-relocate.test.ts @@ -2,7 +2,7 @@ // any mounted catalogue is PLAYED from the catalogue highest in the play // order. The relocation is code's, never the model's — these tests drive // submit_pick directly and read the clip it finishes with. -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { Catalogue, PlayCatalogue, TrackCandidate, TrackPick } from '../src/contracts.ts' import { musicTools } from '../src/music/music-tools.ts' @@ -37,7 +37,7 @@ function build(opts: { { catalogues: () => opts.mounted ?? ['netease', 'bilibili'], onAuthFailure: (err) => auth.push(err), - ...(opts.playOrder !== undefined && { playOrder: opts.playOrder }), + ...(opts.playOrder !== undefined && { playOrder: () => opts.playOrder as PlayCatalogue[] }), debug: (m) => log.push(m), }, ) @@ -138,4 +138,80 @@ describe('play-source preference (spec 14 §2.13)', () => { expect(relocations(provider).map((s) => s.catalogue)).toEqual(['bilibili', 'youtube']) expect(log).toContain('music.relocate from=netease none reason=no-hit') }) + + // Regression, codex review 2026-09-20: a karaoke backing track is the right + // length under the right name, and relocating onto one plays the wrong audio + // under the model's own announce. + it('refuses another recording of the same title', async () => { + const { tools, picks } = build({ + byCatalogue: { + youtube: [candidate('https://www.youtube.com/watch?v=abc', { title: 'Kong Kong - Karaoke instrumental', uploader: 'sing along', durationS: 239 })], + }, + }) + await callTool(tools, 'search_music', { query: 'kong kong', catalogue: 'netease' }) + await submit(tools) + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + }) + + // Regression, codex review 2026-09-20: the artist has to show up somewhere in + // the hit, or a same-titled song by someone else takes the pick. + it('refuses a same-titled hit by someone else', async () => { + const { tools, picks } = build({ + byCatalogue: { youtube: [candidate('https://www.youtube.com/watch?v=abc', { uploader: 'Another Band' })] }, + }) + await submit(tools) + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + // The same hit, with the artist in its uploader, IS taken. + const ok = build({ byCatalogue: { youtube: [candidate('https://www.youtube.com/watch?v=abc', { uploader: 'Chen Li - Topic' })] } }) + await submit(ok.tools) + expect(ok.picks[0]?.clip.source).toBe('https://stream/https://www.youtube.com/watch?v=abc') + }) + + // Regression, codex review 2026-09-20: a channels pick knows its length, so + // the 20 s window must cover that path too — a 3600 s loop version of the + // same title is not the song. + it("keeps a channels candidate's stated length for the window", async () => { + const bili = 'https://www.bilibili.com/video/BV1' + const provider = new FakeMusicProvider() + provider.byCatalogue = { youtube: [candidate('https://www.youtube.com/watch?v=abc', { durationS: 3600 })] } + const picks: TrackPick[] = [] + const tools = musicTools( + provider, + (pick) => picks.push(pick), + async () => true, + { catalogues: () => [], debug: () => {} }, + { count: () => 1, search: () => [candidate(bili, { durationS: 240 })] }, + ) + await callTool(tools, 'search_music', { query: 'kong kong', catalogue: 'channels' }) + await callTool(tools, 'submit_pick', { ref: bili, why: 'w', title: 'Kong Kong', artist: 'Chen Li' }) + expect(picks[0]?.clip.source).toBe(`https://stream/${bili}`) + }) + + // Regression, codex review 2026-09-20: relocation is an optimisation and may + // not become the thing the pick waits on. + it('gives up on a hung relocation search and plays the original', async () => { + const provider = new FakeMusicProvider() + provider.candidates = [candidate(NETEASE_REF)] + provider.byCatalogue = { bilibili: [], qqmusic: [], netease: [candidate(NETEASE_REF)] } + // A search that never settles, exactly as a stuck yt-dlp spawn looks here. + const original = provider.search.bind(provider) + provider.search = async (query, limit, catalogue) => + catalogue === 'youtube' ? new Promise(() => {}) : original(query, limit, catalogue) + const picks: TrackPick[] = [] + const log: string[] = [] + const tools = musicTools(provider, (p) => picks.push(p), async () => true, { + catalogues: () => ['netease', 'bilibili'], + debug: (m) => log.push(m), + }) + vi.useFakeTimers() + try { + const done = callTool(tools, 'submit_pick', { ref: NETEASE_REF, why: 'w', title: 'Kong Kong', artist: 'Chen Li' }) + await vi.advanceTimersByTimeAsync(20_000) + await done + } finally { + vi.useRealTimers() + } + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + expect(log).toContain('music.relocate from=netease none reason=timed-out') + }) }) diff --git a/test/settings.test.ts b/test/settings.test.ts index 57ebf6f..70b7dae 100644 --- a/test/settings.test.ts +++ b/test/settings.test.ts @@ -20,6 +20,7 @@ const BASE: Settings = { muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], } const home = () => mkdtempSync(join(tmpdir(), 'murmur-settings-')) diff --git a/test/setup.test.ts b/test/setup.test.ts index eced161..dff3386 100644 --- a/test/setup.test.ts +++ b/test/setup.test.ts @@ -1299,6 +1299,7 @@ describe('set_language (spec 03-03 \u00a77 + spec 12 \u00a73.9)', () => { muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], } const store = (home: string) => new SettingsStore({ path: join(home, SETTINGS_FILE), initial: BASE, touched: {} }) diff --git a/test/sources-flow.test.ts b/test/sources-flow.test.ts index a8b7317..a1cded5 100644 --- a/test/sources-flow.test.ts +++ b/test/sources-flow.test.ts @@ -16,7 +16,25 @@ import { CHROME_PROFILE_ENV, type ChromeDeps } from '../src/music/sources/chrome import { SourcesStore } from '../src/music/sources/store.ts' import type { SourceId, TasteSnapshot, TasteSource } from '../src/music/sources/taste.ts' import { quitLatch } from '../src/setup/guide.ts' -import { FakeHost } from './fakes.ts' +import { PLAY_ORDER, type TrackPick } from '../src/contracts.ts' +import { musicTools } from '../src/music/music-tools.ts' +import { readSettingsFile, SETTINGS_FILE, SettingsStore } from '../src/host/settings.ts' +import { callTool, FakeHost, FakeMusicProvider } from './fakes.ts' + +// Everything but the knob under test, so a Settings literal here says what it +// is about. +const BASE_SETTINGS = { + anchorsEnabled: true, + musicEnabled: true, + cadenceMode: 'every_n' as const, + musicEveryN: 2, + gapSeconds: 2, + recentWindow: 12, + muted: false, + tuiPet: true, + rwtEnabled: true, + playOrder: [...PLAY_ORDER], +} const NOW = new Date('2026-09-06T12:00:00Z') @@ -913,3 +931,129 @@ describe('runSources (spec 14 §3.1)', () => { expect(SOURCES_OFFER[2]).toContain('/sources') }) }) + +// Where a found song PLAYS from (spec 14 §2.13, acceptance §5.18): the card is +// reached from the /sources menu, a pick moves that catalogue to the front, and +// what it writes is what the next submit_pick reads — no restart in between. +describe('the play order card (spec 14 §2.13/§5.18)', () => { + // The settings authority as the flow sees it, plus a real file behind it so + // the test can read what a listener would still have after a restart. + function withOrder(lines: string[], onWrite?: (host: FakeHost) => void) { + const path = join(mkdtempSync(join(tmpdir(), 'murmur-order-')), SETTINGS_FILE) + const settings = new SettingsStore({ path, initial: { ...BASE_SETTINGS }, touched: {} }) + let written = 0 + const built = build(lines, { + playOrder: { + read: () => settings.current().playOrder, + write: (order) => { + settings.set({ playOrder: [...order] }) + // After the render this write causes, so an Esc here lands on the + // card that is actually up — as a listener's would. + if (++written === 1) setTimeout(() => onWrite?.(built.host), 0) + }, + }, + }) + return { ...built, settings, path } + } + + const options = (host: FakeHost, at: number) => host.asks[at]!.choices!.options! + + it('is an action row on the menu, after refresh', async () => { + const { host, deps, store } = withOrder(['youtube']) + store.mount('youtube', { browser: 'chrome' }) + await runSources(deps) + expect(options(host, 0).at(-1)).toEqual({ + key: 'playOrder', + label: 'play order', + note: 'which catalogue a found song plays from first', + checked: false, + action: true, + }) + expect(options(host, 0).at(-2)?.key).toBe('refresh') + }) + + it('moves a pick to the front, re-renders in place, and comes back with the result', async () => { + // menu -> play order -> Bilibili -> done -> menu -> leave + const { host, deps, store, settings, path } = withOrder(['play order', 'bilibili', 'done', 'bilibili']) + store.mount('bilibili', { auth: 'browser', browser: 'chrome', mid: '9' }) + await runSources(deps) + + // The card opened in the current order, mounted catalogues only, ranked. + expect(options(host, 1)).toEqual([ + { key: 'youtube', label: 'YouTube', note: '1st', checked: true }, + { key: 'bilibili', label: 'Bilibili', note: '2nd', checked: false }, + { key: 'done', label: 'done', note: 'keep this order', action: true }, + ]) + expect(host.asks[1]!.text).toContain('Play from which first? (a pick moves it to the front)') + expect(host.asks[1]!.choices!.multi).toBe(false) + // Re-rendered IN PLACE: the same card again, Bilibili now 1st and ticked, + // led by what the previous pick did. + expect(options(host, 2)).toEqual([ + { key: 'bilibili', label: 'Bilibili', note: '1st', checked: true }, + { key: 'youtube', label: 'YouTube', note: '2nd', checked: false }, + { key: 'done', label: 'done', note: 'keep this order', action: true }, + ]) + expect(host.asks[2]!.text).toContain('ok play order: Bilibili > YouTube') + // `( done )` returns to the menu, which leads with the result. + expect(host.asks[3]!.text).toContain('ok play order: Bilibili > YouTube > QQ Music > NetEase') + // Persisted, hot: the store the pick reads already holds it. + expect(settings.current().playOrder).toEqual(['bilibili', 'youtube', 'qqmusic', 'netease']) + expect(readSettingsFile(path).playOrder).toEqual(['bilibili', 'youtube', 'qqmusic', 'netease']) + }) + + it('shows only mounted catalogues, and keeps an unmounted one in its stored place', async () => { + const { host, deps, store, settings } = withOrder(['play order', 'netease', '', 'netease']) + store.mount('netease', { auth: 'browser', browser: 'chrome', userId: '1', likedPlaylistId: '2' }) + await runSources(deps) + // Bilibili and QQ Music are not mounted, so the card never asks about them. + expect(options(host, 1).map((o) => o.key)).toEqual(['youtube', 'netease', 'done']) + // They keep their stored places behind the promotion all the same. + expect(settings.current().playOrder).toEqual(['netease', 'youtube', 'bilibili', 'qqmusic']) + }) + + it('leaves the order alone when nothing moved', async () => { + const { host, deps } = withOrder(['play order', '', '']) + await runSources(deps) + expect(host.asks[2]!.text).toContain('ok play order unchanged') + }) + + it('Esc abandons the visit and restores the order it opened on', async () => { + // The pick lands, and then the listener presses Esc instead of ( done ). + // No line is queued behind the pick: a queued one would answer the read + // before the Esc could, which is not what a listener's keyboard does. + const { deps, store, settings } = withOrder(['play order', 'bilibili'], (host) => { + host.pressEsc() + // The menu answer comes after the Esc has been read, not queued behind it. + setTimeout(() => host.type('bilibili'), 0) + }) + store.mount('bilibili', { auth: 'browser', browser: 'chrome', mid: '9' }) + await runSources(deps) + expect(settings.current().playOrder).toEqual([...PLAY_ORDER]) + }) + + it('what the card wrote is what the next submit_pick relocates by', async () => { + const { deps, store, settings } = withOrder(['play order', 'bilibili', 'done', 'bilibili']) + store.mount('bilibili', { auth: 'browser', browser: 'chrome', mid: '9' }) + // The pick's tools, built BEFORE the card runs and never rebuilt. + const provider = new FakeMusicProvider() + const hit = (ref: string) => ({ ref, title: 'Kong Kong', uploader: 'Chen Li', durationS: 240, extra: {} }) + provider.byCatalogue = { + youtube: [hit('https://www.youtube.com/watch?v=yt')], + bilibili: [hit('https://www.bilibili.com/video/BVb')], + netease: [hit('https://music.163.com/#/song?id=5')], + } + const picks: TrackPick[] = [] + const tools = musicTools(provider, (p) => picks.push(p), async () => true, { + catalogues: () => ['bilibili', 'netease'], + playOrder: () => settings.current().playOrder, + }) + const submit = () => + callTool(tools, 'submit_pick', { ref: 'https://music.163.com/#/song?id=5', why: 'w', title: 'Kong Kong', artist: 'Chen Li' }) + + await submit() + expect(picks.at(-1)?.clip.source).toContain('watch?v=yt') + await runSources(deps) + await submit() + expect(picks.at(-1)?.clip.source).toContain('video/BVb') + }) +}) diff --git a/test/steer-tools.test.ts b/test/steer-tools.test.ts index bcd2b80..29c7470 100644 --- a/test/steer-tools.test.ts +++ b/test/steer-tools.test.ts @@ -24,6 +24,7 @@ const BASE: Settings = { muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], } function harness(initial: Partial = {}, wired: { music?: boolean } = {}) { diff --git a/test/tui-settings.test.ts b/test/tui-settings.test.ts index a50d2a2..d8325fd 100644 --- a/test/tui-settings.test.ts +++ b/test/tui-settings.test.ts @@ -18,6 +18,7 @@ const VALUES: Settings = { muted: false, tuiPet: true, rwtEnabled: true, + playOrder: ['youtube', 'bilibili', 'qqmusic', 'netease'], } const snap = ( From c1b1a18714bde731ca39e33d7e90b16f79360c5b Mon Sep 17 00:00:00 2001 From: wine-fall <62830944+wine-fall@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:40:49 +0800 Subject: [PATCH 3/3] fix(sources): the play-order card takes the answer the TUI actually sends [spec 14] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every test for the card typed what a plain host types, and the default front-end sends something else: a list answers with the row's own key, and a button press sends the ticks alongside it. So `( play order )` arrived as `youtube playOrder` and was refused as a word the menu could not place, and `( done )` arrived as `youtube done` — the card could be opened only by typing, and left only by Esc, which throws the ordering away. Row keys are now an answer wherever a label or a number is, and the card reads its line as the words it is: any of them naming done, or the row already drawn first, is "keep this order". Three more from the same review, each with its regression test: a marker the submitted title already carries no longer waves every other marker through (asking for a remix and getting "remix - karaoke" is still the wrong recording); a stated length of 0 is yt-dlp's "unknown" and stops refusing every real hit; and a hand-edited settings.json holding a short or repeating order is completed at the boundary, because the card can only promote what it shows and a short list would hide a catalogue with no way back. Co-Authored-By: Claude Opus 5 --- specs/spec14/14-listening-taste.md | 17 ++++++++++++- src/host/ipc.ts | 14 +++++++++-- src/music/music-tools.ts | 14 ++++++++--- src/music/sources/flow.ts | 30 ++++++++++++++++------- test/music-relocate.test.ts | 28 +++++++++++++++++++++ test/sources-flow.test.ts | 39 +++++++++++++++++++++++++++++- 6 files changed, 126 insertions(+), 16 deletions(-) diff --git a/specs/spec14/14-listening-taste.md b/specs/spec14/14-listening-taste.md index cb8c0ae..eec5f0c 100644 --- a/specs/spec14/14-listening-taste.md +++ b/specs/spec14/14-listening-taste.md @@ -1165,7 +1165,22 @@ ok play order: Bilibili > YouTube > QQ Music > NetEase <- the previous pick restored, however many picks were made inside it, and the menu comes back with `ok play order unchanged`. - Plain host: numbers or names, the same one-word-fails-the-line rule as the - menu itself. + menu itself. **A list answers with its ticked row and, on a button press, + that button's key** (spec 10 §3.2-D) — `youtube done`, not `done` — so the + answer is read as the words it is, and the row's own key is an answer + everywhere a label or a number is (*codex review, 2026-09-20: the TUI sends + `playOrder`, which no label or name could match*). +- **The stored order is completed at the settings boundary**: a hand-edited + `settings.json` holding a short or repeating list is filled out in default + order rather than refused, because the card can only promote what it shows + and a short list would hide a catalogue with no way back. + +Matching a relocation candidate carries one more rule than containment and the +window: a hit may carry **no recording marker the submitted title did not** +(karaoke, instrumental, cover, remix, and their Chinese spellings), and the +artist must appear in the hit's title or uploader when one was submitted. A +stated length of `0` is yt-dlp's "unknown", not a length to hold a hit +against. All three fail closed to the original ref. ponytail: promotion is the whole vocabulary — no drag, no move-up/move-down, no rank typing. Three picks put four catalogues in any order, and one key per diff --git a/src/host/ipc.ts b/src/host/ipc.ts index c787269..fd4f0d9 100644 --- a/src/host/ipc.ts +++ b/src/host/ipc.ts @@ -75,6 +75,16 @@ export const MIX_EVERY_N: Record = { export const LANGUAGE_MAX = 40 const LanguageSchema = z.string().trim().min(1).max(LANGUAGE_MAX).regex(/^[^\n\r]+$/) +// The stored order always holds all four catalogues, whatever it is handed +// (spec 14 §2.13): a hand-edited settings.json naming one, or naming one +// twice, is completed in default order rather than refused — the card can +// only promote what it is shown, so a short list would hide a catalogue with +// no way to bring it back. +const PLAY_ORDER_IDS = ['youtube', 'bilibili', 'qqmusic', 'netease'] as const +export const PlayOrderSchema = z + .array(z.enum(PLAY_ORDER_IDS)) + .transform((order) => [...new Set([...order, ...PLAY_ORDER_IDS])]) + export const SettingsValuesSchema = z.object({ anchorsEnabled: z.boolean(), musicEnabled: z.boolean(), @@ -92,7 +102,7 @@ export const SettingsValuesSchema = z.object({ // Where a found song is PLAYED from, best first (spec 14 §2.13). Set from // the /sources play-order card; the ids are spelled out here for the same // reason SourceLine's are — this module ships with the front-end. - playOrder: z.array(z.enum(['youtube', 'bilibili', 'qqmusic', 'netease'])), + playOrder: PlayOrderSchema, // The one OPTIONAL knob (spec 12 §3.9). Absent means the listener never said, // and the persona decides; set is an override applied as a directive on top // of the persona, never an edit to persona.md. Free text — a language name as @@ -113,7 +123,7 @@ export const SettingsPatchSchema = z.object({ muted: z.boolean().optional(), tuiPet: z.boolean().optional(), rwtEnabled: z.boolean().optional(), - playOrder: z.array(z.enum(['youtube', 'bilibili', 'qqmusic', 'netease'])).optional(), + playOrder: PlayOrderSchema.optional(), // Empty string is legal HERE and only here: it is how the listener clears the // override and hands the language back to the persona (spec 12 §3.9). language: z.union([LanguageSchema, z.literal('')]).optional(), diff --git a/src/music/music-tools.ts b/src/music/music-tools.ts index cd2a311..4ca65e3 100644 --- a/src/music/music-tools.ts +++ b/src/music/music-tools.ts @@ -129,7 +129,12 @@ const OTHER_RECORDING = // The Chinese markers are escaped because committed source is English only // (AGENTS.md): \u4f34\u594f backing track, \u7ffb\u5531 cover, // \u7eaf\u97f3\u4e50 instrumental, \u6296\u97f3\u7248 / dj\u7248 edits. - /karaoke|instrumental|cover|remix|nightcore|sped up|slowed|8d audio|\u4f34\u594f|\u7ffb\u5531|\u7eaf\u97f3\u4e50|\u6296\u97f3\u7248|dj\u7248/ + /karaoke|instrumental|cover|remix|nightcore|sped up|slowed|8d audio|\u4f34\u594f|\u7ffb\u5531|\u7eaf\u97f3\u4e50|\u6296\u97f3\u7248|dj\u7248/g + +// Which of those words a title carries. A hit may carry no marker the +// submitted title did not: asking for a remix and getting "remix - karaoke" +// is still the wrong recording (codex review). +const markers = (title: string): Set => new Set(title.match(OTHER_RECORDING) ?? []) // Whoever the model said made it has to show up somewhere in the hit — its // title or its uploader — before the pick is moved onto it. A catalogue that @@ -279,8 +284,11 @@ export function musicTools( const found = folded(hit.title) const sameSong = found.includes(wanted) || wanted.includes(found) if (!sameSong || !sameArtist(hit, artist)) return false - if (OTHER_RECORDING.test(found) && !OTHER_RECORDING.test(wanted)) return false - return length === undefined || Math.abs(hit.durationS - length) <= SAME_LENGTH_S + const asked = markers(wanted) + if ([...markers(found)].some((word) => !asked.has(word))) return false + // yt-dlp prints a missing duration as 0 (`parseSearchOutput`), and an + // unknown length is not a length to hold a hit against. + return length === undefined || length === 0 || Math.abs(hit.durationS - length) <= SAME_LENGTH_S }) if (match === undefined) continue const opened = await deadline.race(openClip(match.ref)).catch((): Opened => ({ ok: false, why: 'dead', error: 'timed out' })) diff --git a/src/music/sources/flow.ts b/src/music/sources/flow.ts index 7c20e0a..243d4fe 100644 --- a/src/music/sources/flow.ts +++ b/src/music/sources/flow.ts @@ -230,7 +230,11 @@ function parsePick(line: string, rows: readonly MenuRow[]): Set | strin for (const word of line.split(/\s+/).filter((w) => w !== '')) { const byNumber = /^\d+$/.test(word) ? rows[Number(word) - 1]?.key : undefined const byLabel = rows.find((row) => row.label.toLowerCase() === word)?.key - const key = byNumber ?? NAMES[word] ?? byLabel + // The TUI answers with the row's own key (`playOrder`), lowercased on the + // way in — a label of two words cannot be matched as one, so the key is + // read directly rather than spelled into NAMES per row (codex review). + const byKey = rows.find((row) => row.key.toLowerCase() === word)?.key + const key = byNumber ?? NAMES[word] ?? byLabel ?? byKey if (key === undefined || !rows.some((row) => row.key === key)) return `I didn't catch "${word}" — numbers or names from the list` picked.add(key) } @@ -509,16 +513,24 @@ async function runPlayOrder( playOrder.write(opened) return 'ok play order unchanged' } - // Enter with the current first still selected, or the done button. - if (line === '' || line === 'done') break - const picked = rows.find( - (row, i) => row.key === line || row.label.toLowerCase() === line || String(i + 1) === line, - ) - if (picked === undefined) { - result = `-- I didn't catch "${line}" — numbers or names from the list` + // A list answers with its ticked row AND, when a button was pressed, that + // button's key — `youtube done`, not `done` (spec 10 §3.2-D). So the line + // is read as the words it is, and the plain host's single word is the + // same read with one word in it. + const words = line.split(/\s+/).filter((w) => w !== '') + const named = words.map((word) => rows.find((row, i) => row.key.toLowerCase() === word || row.label.toLowerCase() === word || String(i + 1) === word)) + const miss = words.find((_, i) => named[i] === undefined) + if (miss !== undefined) { + result = `-- I didn't catch "${miss}" — numbers or names from the list` continue } - if (picked.key === 'done') break + // Enter with nothing, the done button, or the row that is already first: + // three ways of saying "keep this order". + const picked = named.find((row) => row?.key !== 'done') + if (words.length === 0 || named.some((row) => row?.key === 'done')) break + // The row drawn as 1st, which on a card that hides an unmounted catalogue + // is the first one that will actually play. + if (picked === undefined || picked.key === rows[0]?.key) break order = promote(order, picked.key as PlayCatalogue) playOrder.write(order) result = orderLine(order) diff --git a/test/music-relocate.test.ts b/test/music-relocate.test.ts index 4eb433c..1d38c4b 100644 --- a/test/music-relocate.test.ts +++ b/test/music-relocate.test.ts @@ -214,4 +214,32 @@ describe('play-source preference (spec 14 §2.13)', () => { expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) expect(log).toContain('music.relocate from=netease none reason=timed-out') }) + + // Regression, codex review round 2: a marker the submitted title already + // carries must not wave every OTHER marker through with it. + it('refuses a hit that adds a recording marker the pick did not ask for', async () => { + const { tools, picks } = build({ + byCatalogue: { youtube: [candidate('https://www.youtube.com/watch?v=abc', { title: 'Kong Kong (Remix) - Karaoke' })] }, + }) + await callTool(tools, 'submit_pick', { ref: NETEASE_REF, why: 'w', title: 'Kong Kong (Remix)', artist: 'Chen Li' }) + expect(picks[0]?.clip.source).toBe(`https://stream/${NETEASE_REF}`) + // The remix itself is still the song it asked for. + const ok = build({ byCatalogue: { youtube: [candidate('https://www.youtube.com/watch?v=abc', { title: 'Kong Kong (Remix)' })] } }) + await callTool(ok.tools, 'submit_pick', { ref: NETEASE_REF, why: 'w', title: 'Kong Kong (Remix)', artist: 'Chen Li' }) + expect(ok.picks[0]?.clip.source).toContain('watch?v=abc') + }) + + // Regression, codex review round 2: yt-dlp prints a missing duration as 0, + // and a 0 held against a 20 s window refuses every real hit. + it('treats a zero stated length as no length at all', async () => { + const { tools, picks } = build({ + byCatalogue: { + netease: [candidate(NETEASE_REF, { durationS: 0 })], + youtube: [candidate('https://www.youtube.com/watch?v=abc', { durationS: 240 })], + }, + }) + await callTool(tools, 'search_music', { query: 'kong kong', catalogue: 'netease' }) + await submit(tools) + expect(picks[0]?.clip.source).toContain('watch?v=abc') + }) }) diff --git a/test/sources-flow.test.ts b/test/sources-flow.test.ts index a1cded5..0b6aa72 100644 --- a/test/sources-flow.test.ts +++ b/test/sources-flow.test.ts @@ -8,7 +8,7 @@ import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { SourceAuthWatch } from '../src/music/sources/auth.ts' -import { runSources, SOURCES_OFFER, type BrowserMounts, type SourceMounts, type SourcesFlowDeps } from '../src/music/sources/flow.ts' +import { PLAY_ORDER_QUESTION, runSources, SOURCES_OFFER, type BrowserMounts, type SourceMounts, type SourcesFlowDeps } from '../src/music/sources/flow.ts' import { TasteRefresher } from '../src/music/sources/refresh.ts' import { BUNDLED_CLIENT_ID, CLIENT_ID_ENV } from '../src/music/sources/spotify.ts' import { BrowserCookieError } from '../src/music/sources/cookies.ts' @@ -18,6 +18,7 @@ import type { SourceId, TasteSnapshot, TasteSource } from '../src/music/sources/ import { quitLatch } from '../src/setup/guide.ts' import { PLAY_ORDER, type TrackPick } from '../src/contracts.ts' import { musicTools } from '../src/music/music-tools.ts' +import { SettingsValuesSchema } from '../src/host/ipc.ts' import { readSettingsFile, SETTINGS_FILE, SettingsStore } from '../src/host/settings.ts' import { callTool, FakeHost, FakeMusicProvider } from './fakes.ts' @@ -1056,4 +1057,40 @@ describe('the play order card (spec 14 §2.13/§5.18)', () => { await submit() expect(picks.at(-1)?.clip.source).toContain('video/BVb') }) + + // Regression, codex review round 2: the TUI answers with the row's own KEY + // and, on a button press, the ticks alongside it (spec 10 §3.2-D). Every + // test above types what a plain host types, which is the other shape — and + // the shape that was passing while the default front-end could not open the + // card at all. These are the lines `pickAnswer` actually produces. + it('takes the answer the TUI sends, not only the one a plain host types', async () => { + // Pressing ( play order ) on the menu: the ticks plus the row's key. + const { host, deps, store, settings } = withOrder(['youtube playOrder', 'bilibili', 'bilibili done', 'youtube bilibili']) + store.mount('youtube', { browser: 'chrome' }) + store.mount('bilibili', { auth: 'browser', browser: 'chrome', mid: '9' }) + await runSources(deps) + // The card opened (it did not answer "I didn't catch"), the pick landed, + // and ` done` closed it. + expect(host.asks[1]!.text).toContain(PLAY_ORDER_QUESTION) + expect(settings.current().playOrder).toEqual(['bilibili', 'youtube', 'qqmusic', 'netease']) + expect(host.asks.at(-1)!.text).toContain('ok play order: Bilibili > YouTube') + }) + + it('reads the row that is already first as keep-this-order, which is what Enter sends', async () => { + // A single-pick list never answers '': Enter on the ticked first row sends + // that row's key, and taking it as a promotion would never close the card. + const { host, deps, store, settings } = withOrder(['playOrder', 'youtube', 'youtube']) + store.mount('youtube', { browser: 'chrome' }) + await runSources(deps) + expect(host.asks.at(-1)!.text).toContain('ok play order unchanged') + expect(settings.current().playOrder).toEqual([...PLAY_ORDER]) + }) + + // Regression, codex review round 2: a hand-edited settings.json must not be + // able to hide a catalogue from a card that can only promote what it shows. + it('completes a short or repeating stored order at the boundary', () => { + expect(SettingsValuesSchema.shape.playOrder.parse(['bilibili'])).toEqual(['bilibili', 'youtube', 'qqmusic', 'netease']) + expect(SettingsValuesSchema.shape.playOrder.parse([])).toEqual([...PLAY_ORDER]) + expect(SettingsValuesSchema.shape.playOrder.parse(['netease', 'netease'])).toEqual(['netease', 'youtube', 'bilibili', 'qqmusic']) + }) })