Skip to content

Add react-doctor tooling#39

Open
beastyrabbit wants to merge 10 commits intomainfrom
feature/add-react-doctor
Open

Add react-doctor tooling#39
beastyrabbit wants to merge 10 commits intomainfrom
feature/add-react-doctor

Conversation

@beastyrabbit
Copy link
Copy Markdown
Owner

Summary

  • Add react-doctor development tooling

Somewhere a GPU is overheating so I don't have to think.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 22, 2026

Greptile Summary

This PR centralizes all LLM logic to the server layer and adds comprehensive observability tooling ("react-doctor"). The web API routes — previously containing their own LLM calls, prompt construction, and validation — are replaced with thin one-liner proxies to the server via a new autoplayer-proxy.ts utility. The server gains new API endpoints for every autoplayer LLM operation, a substantially refactored prompt-building system, and layers of diagnostic tooling.

Key changes:

  • Architecture consolidation: apps/web/src/services/llm.ts is deleted; all web API routes now proxy to apps/server/src/routes/autoplayer.ts via proxyAutoplayerRequest. This eliminates duplicated prompt logic and ensures server is the single source of truth.
  • Structured prompt builder: llm.ts is refactored from flat string concatenation to a section-based system with PromptProfile (strict/balanced/creative/compact), PromptMode (full/minimal/none), and PromptDistance (close/general/faithful/album) as first-class concepts. Input sanitization (sanitizePromptOptional, sanitizePromptList, sanitizePromptLiteral) is applied consistently to all user-supplied fields.
  • New exported LLM functions: enhancePlaylistPrompt, enhanceSongRequest, refineSessionPrompt, enhanceSessionParams, getSongPromptContract, resolveSongPromptProfile — all backed by tests.
  • Observability: Request lifecycle middleware with request IDs and noisy-route aggregation (index.ts); worker diagnostics snapshots with queue state, oldest in-flight songs, and per-playlist buffer health (worker/index.ts); configurable event bus tracing with slow handler detection (event-bus.ts); structured file logging with sensitive-field redaction (logger.ts).
  • Pipeline truth panel: pipeline-truth.ts + UI additions show live per-stage counts (lyrics/LLM/audio/persona) in the autoplayer footer and queue view.
  • Manager refresh made non-blocking: ensurePlaylistManagerRefresh is now fire-and-forget so song generation starts immediately.

One issue found: the slow event handler warning in event-bus.ts is gated on EVENT_TRACE_ENABLED, which defaults to false in production, making slow handler detection dead code unless LOG_EVENT_BUS is explicitly configured.

Confidence Score: 4/5

  • Safe to merge after the slow-handler warning gate is fixed; no data-loss or security risk.
  • This is a well-structured refactor with clear architectural intent, consistent sanitization throughout the new prompt builder, tests for the new contract API, and thorough observability additions. The one P1 issue (slow event handler warning silenced in production) is a targeted, isolated fix. The P2 inconsistency in buildAlbumPrompt's field sanitization is defense-in-depth and doesn't cause functional breakage since the final assembled prompt is still sanitized.
  • apps/server/src/events/event-bus.ts — slow handler warning is unreachable in production without manual LOG_EVENT_BUS env var.

Important Files Changed

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)

  1. apps/server/src/events/event-bus.ts, line 163-175 (link)

    P1 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:

    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.
  2. apps/server/src/routes/autoplayer.ts, line 2007-2097 (link)

    P2 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 generateSongMetadatabuildSongUserPrompt 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.

    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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant