feat(registry-dash): admin system prompt editor#118
Conversation
- Add GET endpoint to return default system prompt template by page/promptType - Pre-populate Advanced textarea with backend default prompt on toggle - Add localStorage persistence for edited prompts and Reset to Default button - Allow FTE admins to use custom system prompts in all environments (was non-prod only) - Surface Anthropic API error details in 502 responses instead of swallowing them - Fix whitespace-only follow-up input not disabling send button Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an admin-facing system prompt editing workflow for RegistryDash AI analysis, including a new GET endpoint for default prompt templates, frontend prompt persistence/reset UX, and improved surfacing of Anthropic API error details.
Changes:
- Add
GET /console-api/registry-dash/ai/analyzeto return a default system prompt template for a givenpageandpromptType. - Update AI analysis modal to load default prompts, persist edited prompts in
localStorage, and provide a “Reset to Default” control. - Include upstream Anthropic error body in error handling so the UI can display more detailed 502 failures; fix whitespace-only follow-up disabling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/google/registry/ui/server/console/registrydash/RegistryDashAiAction.java | Adds GET handler for default prompt templates; allows FTE prompt overrides; returns SSE error detail on 502. |
| core/src/test/java/google/registry/ui/server/console/registrydash/RegistryDashAiActionTest.java | Updates constructor usage; adds GET handler tests. |
| core/src/main/java/google/registry/ui/server/console/ConsoleModule.java | Adds request parameter providers for aiPage/aiPromptType. |
| core/src/main/java/google/registry/ai/AnthropicClient.java | Includes upstream error body in thrown IOException message for non-2xx responses. |
| console-webapp/src/app/registry-dash/ai/ai-analysis.service.ts | Adds default-prompt fetch; parses 502 SSE body for API error details. |
| console-webapp/src/app/registry-dash/ai/ai-analysis-modal.component.ts | Loads default prompt on Advanced toggle; persists edits; reset-to-default behavior. |
| console-webapp/src/app/registry-dash/ai/ai-analysis-modal.component.html | Adds Reset button and prompt loading UI; trims follow-up disable condition. |
| console-webapp/src/app/registry-dash/ai/ai-analysis-modal.component.scss | Styles for Advanced header/reset/loading/prompt label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!response.isSuccessful()) { | ||
| throw new IOException("Anthropic API error: " + response.code()); | ||
| String errorBody = response.body() != null | ||
| ? response.body().string() : "no response body"; | ||
| throw new IOException( | ||
| "Anthropic API error: " + response.code() + " - " + errorBody); |
There was a problem hiding this comment.
response.body().string() reads the entire error response into memory and the resulting string is propagated into the thrown exception message. If Anthropic returns a large error payload (or HTML), this can unnecessarily inflate memory usage and downstream responses.
Consider capping how much of the error body is read (e.g., via peekBody(maxBytes) or a manual limit) and/or only including a short, sanitized excerpt in the exception.
| if (response.status === 502) { | ||
| try { | ||
| const body = await response.text(); | ||
| const match = body.match(/data: (.+)/); | ||
| if (match) { | ||
| const parsed = JSON.parse(match[1]); | ||
| if (parsed.error) { | ||
| this.error.set(`API error: ${parsed.error}`); | ||
| return; | ||
| } | ||
| } | ||
| } catch { /* fall through */ } |
There was a problem hiding this comment.
The new 502 handling parses only the first data: line via regex and only when response.status === 502. If the backend writes an ErrorChunk after headers have already been committed (status may remain 200), the streaming loop below will currently ignore it because it only handles parsed.text.
Consider making the streaming loop also handle parsed.error chunks (and setting this.error) so error details are surfaced consistently regardless of HTTP status/commit timing.
| const storageKey = `ai-prompt-${this.data.page}-${this.data.promptType}`; | ||
| const saved = localStorage.getItem(storageKey); | ||
| if (saved) { | ||
| this.editableSystemPrompt = saved; | ||
| } |
There was a problem hiding this comment.
Direct localStorage access can throw (e.g., blocked storage / privacy mode), which would break the modal when toggling Advanced or editing the prompt. Since this is optional UX, it should fail closed.
Wrap the getItem/setItem/removeItem calls in a try/catch (or behind a small helper) so prompt editing still works even if persistence is unavailable.
| @Test | ||
| void testGetDefaultPrompt_returnsSystemPrompt() { | ||
| when(params.request().getMethod()).thenReturn("GET"); | ||
|
|
||
| RegistryDashAiAction action = new RegistryDashAiAction( | ||
| params, Optional.empty(), | ||
| Optional.of("domain-activity"), Optional.of("summarize_trends"), | ||
| anthropicClient, rateLimiter); | ||
| action.run(); | ||
|
|
||
| assertThat(response.getStatus()).isEqualTo(200); | ||
| String payload = response.getPayload(); | ||
| assertThat(payload).contains("systemPrompt"); | ||
| assertThat(payload).contains("domain-activity"); | ||
| } |
There was a problem hiding this comment.
There’s no test coverage for the new 502/ErrorChunk behavior when anthropicClient.streamMessage(...) throws an IOException. Adding a unit test that forces an IOException and asserts the 502 status + returned payload format would help prevent regressions (and validate the frontend parsing contract).
| String detail = e.getMessage() != null ? e.getMessage() : ""; | ||
| writer.write("data: " + PLAIN_GSON.toJson( | ||
| new ErrorChunk(detail)) + "\n\n"); | ||
| writer.flush(); |
There was a problem hiding this comment.
The 502 error detail being returned to the browser is derived directly from IOException.getMessage(), which now includes the full upstream Anthropic error body. This can be very large (memory/response-size risk) and may expose upstream/internal details to end users.
Recommend truncating/sanitizing the detail before sending it to the client (e.g., cap length and strip newlines), while logging the full message server-side for debugging.
Summary
Test plan
🤖 Generated with Claude Code