fix: stabilize the live recommendation path
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
bfea9efb5e
commit
dbc5c2d93f
12 changed files with 113 additions and 31 deletions
12
.env.example
12
.env.example
|
|
@ -1,11 +1,11 @@
|
||||||
# Runtime mode; set to demo for no-key fixtures or live for Spotify and Anthropic.
|
# Runtime mode; `demo` works without keys, `live` uses Spotify and Anthropic.
|
||||||
APP_MODE=
|
APP_MODE=demo
|
||||||
|
|
||||||
# Spotify app client ID; required in live mode and unused in demo mode.
|
# Spotify app client ID; required in live mode and unused in demo mode.
|
||||||
SPOTIFY_CLIENT_ID=
|
SPOTIFY_CLIENT_ID=
|
||||||
|
|
||||||
# Spotify OAuth callback URI; set in live mode if the local default is not registered.
|
# Must exactly match the URI registered in Spotify's dashboard.
|
||||||
SPOTIFY_REDIRECT_URI=
|
SPOTIFY_REDIRECT_URI=http://127.0.0.1:8888/callback
|
||||||
|
|
||||||
# Anthropic API key; required in live mode and unused in demo mode.
|
# Anthropic API key; required in live mode and unused in demo mode.
|
||||||
ANTHROPIC_API_KEY=
|
ANTHROPIC_API_KEY=
|
||||||
|
|
@ -13,5 +13,5 @@ ANTHROPIC_API_KEY=
|
||||||
# Pre-authorized Spotify token; only for hosted instances without interactive login.
|
# Pre-authorized Spotify token; only for hosted instances without interactive login.
|
||||||
SPOTIFY_SEED_REFRESH_TOKEN=
|
SPOTIFY_SEED_REFRESH_TOKEN=
|
||||||
|
|
||||||
# Secure session cookie flag; set to true on every HTTPS deployment.
|
# Keep false for local HTTP; set true on every HTTPS deployment.
|
||||||
SESSION_COOKIE_SECURE=
|
SESSION_COOKIE_SECURE=false
|
||||||
|
|
|
||||||
|
|
@ -32,9 +32,10 @@ scenarios; any other input replays the nearest scenario, with a banner
|
||||||
naming it. Playlist writes are simulated and labeled.
|
naming it. Playlist writes are simulated and labeled.
|
||||||
|
|
||||||
**Live with your own keys:** copy `.env.example` to `.env`, set
|
**Live with your own keys:** copy `.env.example` to `.env`, set
|
||||||
`APP_MODE=live`, a Spotify client id (redirect URI
|
`APP_MODE=live`, a Spotify client id and an Anthropic API key, then run
|
||||||
`http://127.0.0.1:8888/callback`) and an Anthropic API key, then
|
`docker compose up --build` and connect Spotify. Register the exact redirect
|
||||||
`docker compose up --build` and log in via Spotify.
|
URI from `.env` in Spotify's dashboard; PKCE needs no client secret. Keep
|
||||||
|
`SESSION_COOKIE_SECURE=false` for local HTTP.
|
||||||
|
|
||||||
## How it works
|
## How it works
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -22,6 +22,7 @@ class ResolvedSession:
|
||||||
|
|
||||||
session_id: str
|
session_id: str
|
||||||
session: SpotifySession
|
session: SpotifySession
|
||||||
|
can_logout: bool
|
||||||
|
|
||||||
|
|
||||||
@router.get("/api/auth/login")
|
@router.get("/api/auth/login")
|
||||||
|
|
@ -81,7 +82,12 @@ def current_session(request: Request) -> JSONResponse:
|
||||||
resolved = resolve_session(request)
|
resolved = resolve_session(request)
|
||||||
if resolved is None:
|
if resolved is None:
|
||||||
return JSONResponse({"detail": "Not authenticated"}, status_code=401)
|
return JSONResponse({"detail": "Not authenticated"}, status_code=401)
|
||||||
return JSONResponse({"display_name": resolved.session.display_name})
|
return JSONResponse(
|
||||||
|
{
|
||||||
|
"display_name": resolved.session.display_name,
|
||||||
|
"can_logout": resolved.can_logout,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.get("/api/suggestions")
|
@router.get("/api/suggestions")
|
||||||
|
|
@ -119,16 +125,16 @@ def resolve_session(request: Request) -> ResolvedSession | None:
|
||||||
application_settings = cast(Settings, request.app.state.settings)
|
application_settings = cast(Settings, request.app.state.settings)
|
||||||
if application_settings.app_mode is AppMode.DEMO:
|
if application_settings.app_mode is AppMode.DEMO:
|
||||||
demo_session = cast(SpotifySession, request.app.state.demo_session)
|
demo_session = cast(SpotifySession, request.app.state.demo_session)
|
||||||
return ResolvedSession("demo", demo_session)
|
return ResolvedSession("demo", demo_session, can_logout=False)
|
||||||
session_store = cast(SessionStore, request.app.state.session_store)
|
session_store = cast(SessionStore, request.app.state.session_store)
|
||||||
cookie_session_id = request.cookies.get(SESSION_COOKIE_NAME)
|
cookie_session_id = request.cookies.get(SESSION_COOKIE_NAME)
|
||||||
if cookie_session_id is not None:
|
if cookie_session_id is not None:
|
||||||
cookie_session = session_store.get(cookie_session_id)
|
cookie_session = session_store.get(cookie_session_id)
|
||||||
if cookie_session is not None:
|
if cookie_session is not None:
|
||||||
return ResolvedSession(cookie_session_id, cookie_session)
|
return ResolvedSession(cookie_session_id, cookie_session, can_logout=True)
|
||||||
|
|
||||||
seed_session_id = cast(str | None, getattr(request.app.state, "seed_session_id", None))
|
seed_session_id = cast(str | None, getattr(request.app.state, "seed_session_id", None))
|
||||||
seed_session = session_store.get(seed_session_id) if seed_session_id is not None else None
|
seed_session = session_store.get(seed_session_id) if seed_session_id is not None else None
|
||||||
if seed_session is None or seed_session_id is None:
|
if seed_session is None or seed_session_id is None:
|
||||||
return None
|
return None
|
||||||
return ResolvedSession(seed_session_id, seed_session)
|
return ResolvedSession(seed_session_id, seed_session, can_logout=False)
|
||||||
|
|
|
||||||
|
|
@ -64,8 +64,12 @@ Retain understandable bridges through genre, scene, production style, instrument
|
||||||
energy, era, or songwriting. The profile is not exhaustive, so never claim that a
|
energy, era, or songwriting. The profile is not exhaustive, so never claim that a
|
||||||
candidate is definitely unknown. When familiarity is
|
candidate is definitely unknown. When familiarity is
|
||||||
familiar, candidates may include supplied top or saved tracks, but remain responsive to the
|
familiar, candidates may include supplied top or saved tracks, but remain responsive to the
|
||||||
current request. When familiarity is mix, combine recognizable anchors with real,
|
current request. When familiarity is mix, the first 15 candidates must be suitable anchors
|
||||||
confidently identifiable adjacent discoveries rather than splitting into unrelated halves.
|
copied from the supplied top or saved tracks: copy the title exactly and use one listed
|
||||||
|
artist name exactly, preferring the first. Deduplicate those anchors, then fill the remaining
|
||||||
|
candidates with confidently identifiable adjacent discoveries. If fewer than 15 supplied
|
||||||
|
tracks fit, use every suitable one before adding discoveries. When familiarity is familiar,
|
||||||
|
use supplied tracks for the majority of the list.
|
||||||
|
|
||||||
Use musical knowledge conservatively. Base selection on durable, commonly knowable
|
Use musical knowledge conservatively. Base selection on durable, commonly knowable
|
||||||
attributes of recordings. Do not invent listening statistics, personal memories, release
|
attributes of recordings. Do not invent listening statistics, personal memories, release
|
||||||
|
|
|
||||||
|
|
@ -52,7 +52,10 @@ def test_login_callback_cookie_and_current_user_flow() -> None:
|
||||||
assert "discovery_session=" in callback_response.headers["set-cookie"]
|
assert "discovery_session=" in callback_response.headers["set-cookie"]
|
||||||
assert "HttpOnly" in callback_response.headers["set-cookie"]
|
assert "HttpOnly" in callback_response.headers["set-cookie"]
|
||||||
assert "SameSite=lax" in callback_response.headers["set-cookie"]
|
assert "SameSite=lax" in callback_response.headers["set-cookie"]
|
||||||
assert client.get("/api/auth/me").json() == {"display_name": "Ada Listener"}
|
assert client.get("/api/auth/me").json() == {
|
||||||
|
"display_name": "Ada Listener",
|
||||||
|
"can_logout": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def test_unknown_callback_state_redirects_to_login_error() -> None:
|
def test_unknown_callback_state_redirects_to_login_error() -> None:
|
||||||
|
|
@ -108,9 +111,16 @@ def test_seed_session_authenticates_requests_without_a_cookie() -> None:
|
||||||
)
|
)
|
||||||
with TestClient(app) as client:
|
with TestClient(app) as client:
|
||||||
response = client.get("/api/auth/me")
|
response = client.get("/api/auth/me")
|
||||||
|
logout_response = client.post("/api/auth/logout")
|
||||||
|
after_logout_response = client.get("/api/auth/me")
|
||||||
|
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
assert response.json() == {"display_name": "Seed Listener"}
|
assert response.json() == {"display_name": "Seed Listener", "can_logout": False}
|
||||||
|
assert logout_response.status_code == 204
|
||||||
|
assert after_logout_response.json() == {
|
||||||
|
"display_name": "Seed Listener",
|
||||||
|
"can_logout": False,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def test_seed_session_failure_keeps_application_serving() -> None:
|
def test_seed_session_failure_keeps_application_serving() -> None:
|
||||||
|
|
|
||||||
|
|
@ -195,7 +195,7 @@ def test_demo_auth_playlist_and_suggestions_are_explicitly_simulated() -> None:
|
||||||
for scenario in load_scenarios()
|
for scenario in load_scenarios()
|
||||||
if not scenario.is_refinement
|
if not scenario.is_refinement
|
||||||
]
|
]
|
||||||
assert current_user.json() == {"display_name": "Demo Listener"}
|
assert current_user.json() == {"display_name": "Demo Listener", "can_logout": False}
|
||||||
assert login.headers["location"] == "/?login=demo"
|
assert login.headers["location"] == "/?login=demo"
|
||||||
assert playlist.json()["url"].startswith("https://open.spotify.com/playlist/demo-")
|
assert playlist.json()["url"].startswith("https://open.spotify.com/playlist/demo-")
|
||||||
assert suggestions.json() == expected_suggestions
|
assert suggestions.json() == expected_suggestions
|
||||||
|
|
|
||||||
|
|
@ -526,15 +526,29 @@ Wat ik deed:
|
||||||
- De eerdere aanname van een dagelijks resetmoment gecorrigeerd. Spotify deelt
|
- De eerdere aanname van een dagelijks resetmoment gecorrigeerd. Spotify deelt
|
||||||
Development Mode quota per developer-account en endpoint bucket, maar
|
Development Mode quota per developer-account en endpoint bucket, maar
|
||||||
publiceert de limieten en het reset window niet.
|
publiceert de limieten en het reset window niet.
|
||||||
|
- De gecontroleerde live-test hield door een slechte candidate call maar 1 van
|
||||||
|
35 tracks over. Voor `mix` beginnen nu 15 candidates als exact gekopieerde
|
||||||
|
taste-profile anchors; dezelfde test gaf daarna 17 grounded tracks, 15 cards
|
||||||
|
en een werkende playlist write.
|
||||||
|
- `.env.example` copy-safe gemaakt voor local live mode: een geldige demo
|
||||||
|
default, de exacte loopback redirect URI en secure cookies uit op HTTP.
|
||||||
|
- De gehoste seed session toont geen logout meer: die gedeelde identity wordt
|
||||||
|
bij startup gezet en heeft geen interactieve login flow. De demo-link in de
|
||||||
|
header gebruikt nu dezelfde button style als Dev.
|
||||||
|
|
||||||
Waarom:
|
Waarom:
|
||||||
|
|
||||||
- Een quota error is geen geldige zoekopdracht met nul resultaten. Door deze
|
- Een quota error is geen geldige zoekopdracht met nul resultaten. Door deze
|
||||||
rate-limit issues lever ik op 12 augustus in: eerst één gecontroleerde
|
rate-limit issues lever ik op 12 augustus in: eerst één gecontroleerde
|
||||||
live-test, daarna direct de hand-in.
|
live-test, daarna direct de hand-in.
|
||||||
|
- Grounding moet hallucinations tegenhouden, maar met genoeg echte tracks in
|
||||||
|
het taste profile mag één slechte LLM call niet eindigen in één card.
|
||||||
|
|
||||||
Wat ik heb laten vallen of uitgesteld:
|
Wat ik heb laten vallen of uitgesteld:
|
||||||
|
|
||||||
- De structurele oplossing is een Spotify key met hogere quota. Dat is voor
|
- De structurele oplossing voor quota is een Spotify key met hogere quota. Dat
|
||||||
deze PoC-assessment niet proportioneel; ik bouw geen slimme workaround en
|
is voor deze PoC-assessment niet proportioneel; live mode valt niet stil terug
|
||||||
live mode valt niet stil terug op fixtures.
|
op fixtures.
|
||||||
|
- Geen bounded retry na een dunne pool: die kon de 75 Spotify calls verdubbelen
|
||||||
|
en voegde een nieuwe control flow toe. De prompt gebruikt eerst de data die
|
||||||
|
al in het taste profile staat.
|
||||||
|
|
|
||||||
|
|
@ -48,7 +48,7 @@ const modeLabel = computed(() => {
|
||||||
<span class="mode" :class="{ failed: healthFailed }">
|
<span class="mode" :class="{ failed: healthFailed }">
|
||||||
{{ modeLabel }}
|
{{ modeLabel }}
|
||||||
</span>
|
</span>
|
||||||
<a v-if="demoUrl" class="demo-link" :href="demoUrl">
|
<a v-if="demoUrl" class="button demo-link" :href="demoUrl">
|
||||||
{{ messages.appHeaderDemoInstance }}
|
{{ messages.appHeaderDemoInstance }}
|
||||||
</a>
|
</a>
|
||||||
<AuthStatus :auth="auth" @logout="$emit('logout')" />
|
<AuthStatus :auth="auth" @logout="$emit('logout')" />
|
||||||
|
|
@ -146,21 +146,19 @@ const modeLabel = computed(() => {
|
||||||
}
|
}
|
||||||
|
|
||||||
.demo-link {
|
.demo-link {
|
||||||
color: var(--c-text-chip);
|
|
||||||
text-transform: uppercase;
|
text-transform: uppercase;
|
||||||
text-decoration: none;
|
text-decoration: none;
|
||||||
border-bottom: 1px solid var(--c-line-strong);
|
|
||||||
}
|
|
||||||
|
|
||||||
.demo-link:hover {
|
|
||||||
color: var(--c-text);
|
|
||||||
border-color: var(--c-accent);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
.button {
|
.button {
|
||||||
|
display: inline-flex;
|
||||||
|
flex: none;
|
||||||
|
align-items: center;
|
||||||
min-height: 31px;
|
min-height: 31px;
|
||||||
padding: 6px 11px;
|
padding: 6px 11px;
|
||||||
color: var(--c-text-muted);
|
color: var(--c-text-muted);
|
||||||
|
font-family: var(--f-mono);
|
||||||
|
font-size: var(--t-micro);
|
||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
background: transparent;
|
background: transparent;
|
||||||
border: 1px solid var(--c-line);
|
border: 1px solid var(--c-line);
|
||||||
|
|
@ -169,6 +167,7 @@ const modeLabel = computed(() => {
|
||||||
|
|
||||||
.button:hover {
|
.button:hover {
|
||||||
color: var(--c-text);
|
color: var(--c-text);
|
||||||
|
text-decoration: none;
|
||||||
border-color: var(--c-focus-border);
|
border-color: var(--c-focus-border);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@ const logoutLabel = computed(() => {
|
||||||
>{{ auth.user.display_name }}
|
>{{ auth.user.display_name }}
|
||||||
</span>
|
</span>
|
||||||
<button
|
<button
|
||||||
|
v-if="auth.user.can_logout"
|
||||||
class="button"
|
class="button"
|
||||||
type="button"
|
type="button"
|
||||||
:disabled="auth.status === 'logging_out'"
|
:disabled="auth.status === 'logging_out'"
|
||||||
|
|
|
||||||
|
|
@ -9,10 +9,14 @@ function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
}
|
}
|
||||||
|
|
||||||
function parseCurrentUser(value: unknown): CurrentUser {
|
function parseCurrentUser(value: unknown): CurrentUser {
|
||||||
if (!isRecord(value) || typeof value.display_name !== 'string') {
|
if (
|
||||||
|
!isRecord(value) ||
|
||||||
|
typeof value.display_name !== 'string' ||
|
||||||
|
typeof value.can_logout !== 'boolean'
|
||||||
|
) {
|
||||||
throw new Error(messages.useApiInvalidCurrentUser)
|
throw new Error(messages.useApiInvalidCurrentUser)
|
||||||
}
|
}
|
||||||
return { display_name: value.display_name }
|
return { display_name: value.display_name, can_logout: value.can_logout }
|
||||||
}
|
}
|
||||||
|
|
||||||
function parsePlaylistResponse(value: unknown): { url: string | null } {
|
function parsePlaylistResponse(value: unknown): { url: string | null } {
|
||||||
|
|
|
||||||
|
|
@ -76,4 +76,5 @@ export interface PlaylistCreateResponse {
|
||||||
|
|
||||||
export interface CurrentUser {
|
export interface CurrentUser {
|
||||||
display_name: string
|
display_name: string
|
||||||
|
can_logout: boolean
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -85,6 +85,48 @@ describe('result presentation', () => {
|
||||||
expect(onReset).toHaveBeenCalledOnce()
|
expect(onReset).toHaveBeenCalledOnce()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('renders demo as a header button and hides logout for a seeded user', () => {
|
||||||
|
const demoUrl = 'https://demo.example.com'
|
||||||
|
const root = mountComponent(AppHeader, {
|
||||||
|
auth: {
|
||||||
|
status: 'authenticated',
|
||||||
|
user: { display_name: 'Seed Listener', can_logout: false },
|
||||||
|
message: null,
|
||||||
|
},
|
||||||
|
mode: 'live',
|
||||||
|
healthFailed: false,
|
||||||
|
demoUrl,
|
||||||
|
})
|
||||||
|
const demoLink = root.querySelector<HTMLAnchorElement>('a.demo-link')
|
||||||
|
|
||||||
|
expect(demoLink?.classList.contains('button')).toBe(true)
|
||||||
|
expect(demoLink?.href).toBe(`${demoUrl}/`)
|
||||||
|
expect(demoLink?.textContent).toContain('demo')
|
||||||
|
expect(root.textContent).toContain('Seed Listener')
|
||||||
|
expect(root.textContent).not.toContain('Log out')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('keeps logout available for an interactive local user', () => {
|
||||||
|
const onLogout = vi.fn()
|
||||||
|
const root = mountComponent(AppHeader, {
|
||||||
|
auth: {
|
||||||
|
status: 'authenticated',
|
||||||
|
user: { display_name: 'Local Listener', can_logout: true },
|
||||||
|
message: null,
|
||||||
|
},
|
||||||
|
mode: 'live',
|
||||||
|
healthFailed: false,
|
||||||
|
demoUrl: null,
|
||||||
|
onLogout,
|
||||||
|
})
|
||||||
|
const logout = [...root.querySelectorAll<HTMLButtonElement>('button')].find((button) =>
|
||||||
|
button.textContent?.includes('Log out'),
|
||||||
|
)
|
||||||
|
|
||||||
|
logout?.click()
|
||||||
|
expect(onLogout).toHaveBeenCalledOnce()
|
||||||
|
})
|
||||||
|
|
||||||
it('keeps completion context outside the focusable track region', () => {
|
it('keeps completion context outside the focusable track region', () => {
|
||||||
const root = mountComponent(AssistantMessage, { turn: doneTurn(), canSave: true })
|
const root = mountComponent(AssistantMessage, { turn: doneTurn(), canSave: true })
|
||||||
const region = root.querySelector<HTMLElement>(
|
const region = root.querySelector<HTMLElement>(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue