Conversation
|
| Filename | Overview |
|---|---|
| apps/server/src/external/llm.ts | Major refactor: flat string prompt construction replaced with structured section builder, input sanitization added for all user-supplied fields, new exported functions (enhancePlaylistPrompt, enhanceSongRequest, refineSessionPrompt, enhanceSessionParams, getSongPromptContract, resolveSongPromptProfile), temperature now profile-driven, prompt diagnostics logging added. Well structured and internally consistent. |
| apps/server/src/routes/autoplayer.ts | New API endpoints added for all LLM operations (generate-song, generate-album-track, extract-persona, enhance-prompt, enhance-request, refine-prompt, enhance-session, prompt-contract). Input parsing is consistent via helper functions. However, buildAlbumPrompt does not apply the sanitizePromptOptional helpers used in the llm.ts prompt builders, creating an inconsistency in how request body fields are embedded into LLM prompts. |
| apps/server/src/events/event-bus.ts | Adds configurable event tracing with sequence numbers, listener counts, payload logging, and slow handler detection. The slow handler warning is gated inside EVENT_TRACE_ENABLED which defaults to false in production, meaning slow handlers will never be flagged unless LOG_EVENT_BUS is explicitly set. |
| apps/server/src/logger.ts | Upgraded to multi-transport logging: pino-pretty for dev, pino/file for NDJSON log files. Adds sensitive field redaction (authorization, apiKey, openrouterApiKey), service/logSessionId base fields, and single-line pretty option. File logging disabled in test environments. Exports loggingConfig for startup banner. |
| apps/server/src/index.ts | Adds request lifecycle middleware with requestId generation/propagation, slow request detection, aggregated logging for noisy polling routes, and unhandledRejection/uncaughtException process handlers. Graceful shutdown now clears the noisy request summary timer and stops worker diagnostics. |
| apps/server/src/worker/index.ts | Adds periodic worker diagnostics snapshots: queue state summaries, song status counts by state from SQLite, oldest transient song detection, and per-playlist buffer health checks. Long-running queue items and low-buffer playlists trigger targeted warnings. Adds stopWorkerDiagnostics export for graceful shutdown. |
| apps/web/src/lib/autoplayer-proxy.ts | New thin proxy utility that forwards web API route requests to the server's /api/autoplayer endpoint. Propagates query string, content-type header, and request body. Forwards upstream status code. Clean and minimal. |
| apps/web/src/lib/pipeline-truth.ts | New utility that joins song status list with worker queue state to produce a unified pipeline truth view (lyricsInProgress, lyricsInQueue, lyricsPreQueue, audioInProgress, audioInQueue, personaLlmJobs etc.). Clean separation of concerns used by both the autoplayer page footer and the queue view. |
| apps/server/src/worker/song-worker.ts | Manager refresh extracted to ensurePlaylistManagerRefresh() and changed from awaited to fire-and-forget. Comment explains the intent: best-effort, out-of-band so song generation starts immediately. The previous aborted-check after the refresh is removed, which is correct since the refresh is now non-blocking. |
| apps/server/src/tests/prompt-contract.test.ts | New test file covering getSongPromptContract and resolveSongPromptProfile. Tests mode/profile defaults, compact vs. full prompt length, explicit none mode, faithful-to-strict mapping, faithful+exploration-keywords relaxation, and explicit profile override. Good coverage of the new contract API. |
Comments Outside Diff (2)
-
apps/server/src/events/event-bus.ts, line 163-175 (link)Slow-handler warning silenced in production
The
"Event handler slow"warning is nested insideif (EVENT_TRACE_ENABLED), butEVENT_TRACE_ENABLEDdefaults toisDev— which isfalsein production (unlessLOG_EVENT_BUSis explicitly set). This means slow event handlers in production will never emit any observability signal, defeating the purpose of the diagnostic.The timing measurement (
startedAt) is always recorded but the check only runs when tracing is on. The slow-warn check should be independent of the trace flag so it fires in production without requiringLOG_EVENT_BUS:Prompt To Fix With AI
This is a comment left during a code review. Path: apps/server/src/events/event-bus.ts Line: 163-175 Comment: **Slow-handler warning silenced in production** The `"Event handler slow"` warning is nested inside `if (EVENT_TRACE_ENABLED)`, but `EVENT_TRACE_ENABLED` defaults to `isDev` — which is `false` in production (unless `LOG_EVENT_BUS` is explicitly set). This means slow event handlers in production will never emit any observability signal, defeating the purpose of the diagnostic. The timing measurement (`startedAt`) is always recorded but the check only runs when tracing is on. The slow-warn check should be independent of the trace flag so it fires in production without requiring `LOG_EVENT_BUS`: How can I resolve this? If you propose a fix, please make it concise.
-
apps/server/src/routes/autoplayer.ts, line 2007-2097 (link)buildAlbumPromptembeds unsanitized request fields into LLM promptEvery other prompt-building function in
llm.ts(e.g.,buildPersonaUserPrompt,buildSongUserPrompt,generatePlaylistManagerPlan) callssanitizePromptOptionalorsanitizePromptListon each field before embedding it.buildAlbumPromptskips this step entirely and embeds user-supplied values —source.title,source.artistName,source.description,source.lyrics,personaExtracts,avoidPersonaExtracts, etc. — directly into the final prompt string.Although
generateSongMetadata→buildSongUserPrompteventually callssanitizePromptOptionalon the fully assembled album prompt (stripping control/format characters), this only provides sanitization at the outermost layer. Each embedded field is already fully expanded into the string by the time sanitization occurs. The intent of the sanitization helpers is to guard field boundaries, not just the final concatenated blob.This inconsistency is worth aligning for defense-in-depth, especially for
personaExtractsandavoidPersonaExtractswhich are ultimately derived from LLM output that the user may have influenced. At minimum, sanitizesource.title,source.artistName,source.description,source.lyrics, and elements frompersonaExtracts/avoidPersonaExtractsusing the samesanitizePromptOptionalhelpers used elsewhere.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/server/src/routes/autoplayer.ts Line: 2007-2097 Comment: **`buildAlbumPrompt` embeds unsanitized request fields into LLM prompt** Every other prompt-building function in `llm.ts` (e.g., `buildPersonaUserPrompt`, `buildSongUserPrompt`, `generatePlaylistManagerPlan`) calls `sanitizePromptOptional` or `sanitizePromptList` on each field before embedding it. `buildAlbumPrompt` skips this step entirely and embeds user-supplied values — `source.title`, `source.artistName`, `source.description`, `source.lyrics`, `personaExtracts`, `avoidPersonaExtracts`, etc. — directly into the final prompt string. Although `generateSongMetadata` → `buildSongUserPrompt` eventually calls `sanitizePromptOptional` on the fully assembled album prompt (stripping control/format characters), this only provides sanitization at the outermost layer. Each embedded field is already fully expanded into the string by the time sanitization occurs. The intent of the sanitization helpers is to guard field boundaries, not just the final concatenated blob. This inconsistency is worth aligning for defense-in-depth, especially for `personaExtracts` and `avoidPersonaExtracts` which are ultimately derived from LLM output that the user may have influenced. At minimum, sanitize `source.title`, `source.artistName`, `source.description`, `source.lyrics`, and elements from `personaExtracts` / `avoidPersonaExtracts` using the same `sanitizePromptOptional` helpers used elsewhere. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/server/src/events/event-bus.ts
Line: 163-175
Comment:
**Slow-handler warning silenced in production**
The `"Event handler slow"` warning is nested inside `if (EVENT_TRACE_ENABLED)`, but `EVENT_TRACE_ENABLED` defaults to `isDev` — which is `false` in production (unless `LOG_EVENT_BUS` is explicitly set). This means slow event handlers in production will never emit any observability signal, defeating the purpose of the diagnostic.
The timing measurement (`startedAt`) is always recorded but the check only runs when tracing is on. The slow-warn check should be independent of the trace flag so it fires in production without requiring `LOG_EVENT_BUS`:
```suggestion
try {
await handler(data);
const elapsedMs = Date.now() - startedAt;
if (elapsedMs >= EVENT_HANDLER_SLOW_MS) {
logger.warn(
{ event, sequence, elapsedMs, slowMs: EVENT_HANDLER_SLOW_MS },
"Event handler slow",
);
} else if (EVENT_TRACE_ENABLED && EVENT_HANDLER_TRACE_ENABLED) {
logger.debug({ event, sequence, elapsedMs }, "Event handled");
}
} catch (err) {
logger.error({ err, event }, "Event handler error");
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/server/src/routes/autoplayer.ts
Line: 2007-2097
Comment:
**`buildAlbumPrompt` embeds unsanitized request fields into LLM prompt**
Every other prompt-building function in `llm.ts` (e.g., `buildPersonaUserPrompt`, `buildSongUserPrompt`, `generatePlaylistManagerPlan`) calls `sanitizePromptOptional` or `sanitizePromptList` on each field before embedding it. `buildAlbumPrompt` skips this step entirely and embeds user-supplied values — `source.title`, `source.artistName`, `source.description`, `source.lyrics`, `personaExtracts`, `avoidPersonaExtracts`, etc. — directly into the final prompt string.
Although `generateSongMetadata` → `buildSongUserPrompt` eventually calls `sanitizePromptOptional` on the fully assembled album prompt (stripping control/format characters), this only provides sanitization at the outermost layer. Each embedded field is already fully expanded into the string by the time sanitization occurs. The intent of the sanitization helpers is to guard field boundaries, not just the final concatenated blob.
This inconsistency is worth aligning for defense-in-depth, especially for `personaExtracts` and `avoidPersonaExtracts` which are ultimately derived from LLM output that the user may have influenced. At minimum, sanitize `source.title`, `source.artistName`, `source.description`, `source.lyrics`, and elements from `personaExtracts` / `avoidPersonaExtracts` using the same `sanitizePromptOptional` helpers used elsewhere.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore: add react-doctor tooling" | Re-trigger Greptile
Summary
Somewhere a GPU is overheating so I don't have to think.