feat(ai): compact structured output for /explain and /optimize#747
feat(ai): compact structured output for /explain and /optimize#747
Conversation
REV Review — fix/742-ai-compact-outputAPPROVE (one minor fix recommended) What changed
One issue: max_tokens overrides user config// Current (overrides user config silently):
max_tokens: 400,
// Better (caps at 400 but respects lower user configs):
max_tokens: settings.config.ai.max_tokens.min(400),Same for optimize: A user who set This is a minor issue, not a blocker. Fix it before merge if possible. No other issues
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #747 +/- ##
==========================================
+ Coverage 69.10% 69.11% +0.01%
==========================================
Files 47 47
Lines 31990 32000 +10
==========================================
+ Hits 22105 22115 +10
Misses 9885 9885 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Code Review: PR #747 — feat(ai): compact structured output for /explain and /optimizeSummaryGood direction — structured output reduces token waste and makes responses more parseable. Two real concerns worth fixing before merge. BLOCKINGB1:
|
Testing Evidence — fix/742-ai-compact-outputCommit: f597636 feat(ai): compact structured output for /explain and /optimize (#742) Prompt verification
max_tokens values: Test resultsUnit test suite: 1811 passed; 0 failed (finished in 1.64s) Integration smoke tests: 28 skipped — all require live Postgres on port 15432 (expected, not available in this environment). The tests that could run passed (integration_repl: 18 passed; 0 failed). New prompt testsBoth new prompt validation tests pass. The |
|
Fixed all three REV findings in one commit (2097a66):
|
REV Code Review Report
CI Status: ✅ All checks pass (CodeQL, lint, tests, coverage, builds) BLOCKING ISSUES (0)No blocking issues found. NON-BLOCKING (3)INFO
LOW
INFO Commit subjects exceed 50-char guideline
POTENTIAL ISSUES (1)MEDIUM
Summary
Note:
REV-assisted review (AI analysis by postgres-ai/rev) |
1 similar comment
REV Code Review Report
CI Status: ✅ All checks pass (CodeQL, lint, tests, coverage, builds) BLOCKING ISSUES (0)No blocking issues found. NON-BLOCKING (3)INFO
LOW
INFO Commit subjects exceed 50-char guideline
POTENTIAL ISSUES (1)MEDIUM
Summary
Note:
REV-assisted review (AI analysis by postgres-ai/rev) |
REV Code Review — PR #747PR: #747 — feat(ai): compact structured output for /explain and /optimize BLOCKING ISSUESNone. NON-BLOCKINGLOW
LOW
POTENTIAL ISSUESMEDIUM
MEDIUM
Summary
Compact output is a meaningful UX improvement — the structured labeled-line format for REV-assisted review (AI analysis by postgres-ai/rev) |
|
REV PR #747 — feat(ai): compact structured output for /explain and /optimize Summary: Rewrites the system prompts for Observations:
Verdict: Solid prompt engineering. Points 1 and 5 worth a quick check. No blockers. |
|
REV #747 — feat(ai): compact structured output for /explain and /optimizeOverall: solid improvement. Structured labeled-line output is a meaningful UX win over prose — scannable at a glance, fits narrow terminals. The max_tokens cap is sensible. What's good
Concerns
Minor
Verdict: approve with nits. Not blocking. |
REV Code Review — PR #747: feat(ai): compact structured output for /explain and /optimizeCI: green | Diff scope: Blocking findingsNone. Non-blocking findings[BUG / MEDIUM] The [BEHAVIOR / LOW] If a user has configured [BEHAVIOR / LOW] The system prompt says "CREATE INDEX items must be valid SQL: CREATE INDEX CONCURRENTLY name ON table(col);" — this is good practice, but [STYLE / LOW] Two self-referential tests that use fn explain_system_prompt_is_structured() {
let prompt_fragment = "Bottleneck:";
assert!(include_str!("ai_commands.rs").contains(prompt_fragment), ...);
}These tests verify that the source file contains a literal string. This is a "change detector" test — it passes as long as the code compiles and the string exists, but it does not test behavior. If the prompt changes to use a different label, the test fails with a confusing message. Better: extract the system prompt content into a Potential issues (low confidence)[BEHAVIOR] Structured output format depends entirely on AI model compliance The new prompts use imperative instructions ("Output ONLY the labeled lines", "No prose intro, no section headers, no markdown") but AI models can deviate, especially on edge-case inputs. There is no parsing or validation of the AI response — the output is printed verbatim. If the model ignores the format instructions, the user gets prose instead of structured output. This is inherent to prompt-based formatting; acceptable for now, but worth a comment that structured output is best-effort. SummaryThe compact output format is a meaningful UX improvement — structured labeled lines are much more scannable than prose paragraphs. The |
|
CI is green and REV review found no blocking findings. This PR is ready for merge — awaiting Nik's approval. |
|
REV PR #747 — feat(ai): compact structured output for /explain and /optimize Good direction — structured labeled-line output for Observations:
Nothing blocking. The self-referential test issue (#1) is the main thing worth addressing before merge. |
Testing Evidence — PR #747Tested on branch CI StatusAll 16 checks pass: /explain — sequential scan with missing indexCommand: EXPLAIN ANALYZE output (shown verbatim by rpg): AI compact output (4 lines, max 6): 4 labeled lines. No prose. No markdown headers. Empty labels omitted. /explain — index scan (fast path)Command: AI compact output (1 line — nothing significant to report for other labels): Demonstrates that irrelevant labels are omitted entirely (Bottleneck/Filters/Slow nodes suppressed for a trivial 0.058ms index scan). /optimize — same sequential scanCommand: AI compact output (numbered list, 6 items max): 6 numbered items. Item 1 is valid SQL ( Summary
Compact format confirmed working. All findings preserved. All CI checks green. |
Fix verified: commit 31e6b3bProblemThe FixTightened the system prompt in
Before (old prompt rules)After (new prompt rules)Live terminal output (ashtest DB, gpt-4o-mini)Verified: output contains none of Checks
|
Closes #742
Replaces verbose prose AI output with compact structured format for terminal use. All findings preserved.
/explain output format:
/optimize output format: