REV Code Review — PR #755: feat(repl): establish /cmd convention for rpg-specific commands

CI: FAILING (rustfmt) | Diff scope: CLAUDE.md, COMMANDS.md (new), src/repl/ai_commands.rs

Blocking findings

[CI / BLOCKING] rustfmt failure on src/repl/ai_commands.rs

The lint step fails at cargo fmt --check with:

-            crate::dba::execute(client, &format!("ash {rest}").trim().to_owned(), false, Some(&caps), settings).await;
+            &format!("ash {rest}").trim().to_owned(),

The /ash handler has a long crate::dba::execute(...) call that rustfmt wants broken across multiple lines. Run cargo fmt on the branch to fix. This is the only blocker.

Non-blocking findings

[BUG / MEDIUM] dispatch_ai_command returns Some(MetaResult) for mode-change commands but the handle_line caller only handles Reconnected and Quit

In handle_line:

if let Some(result) = dispatch_ai_command(...).await {
    match result {
        MetaResult::Reconnected(c, p) => return HandleLineResult::Reconnected(c, p),
        MetaResult::Quit => return HandleLineResult::Quit,
        _ => {}
    }
}

The /sql, /text2sql, /plan, /yolo, /interactive, /mode branches inside dispatch_ai_command call apply_mode_change or print to stderr and return None. They do not return a MetaResult — they handle the state change internally and return None. This is correct. However, the session-related branches (/session resume) return Some(MetaResult::Reconnected(...)) via dispatch_session_resume. Confirm that all paths that need to propagate a result do so via Some(...) and that apply_mode_change side effects in settings are sufficient (they are, since settings is &mut). This appears correct but the control flow is subtle enough to warrant a comment.

[BEHAVIOR / LOW] /refresh inside dispatch_ai_command prints \refresh is deprecated on the \refresh path but /refresh in the new / path would hit the MetaCmd::RefreshSchema arm, not the AI dispatch

The deprecation eprintln on MetaCmd::RefreshSchema is in dispatch_meta (the \ path). The new /refresh handler in dispatch_ai_command does not have a deprecation message — which is correct. But check that /refresh is actually routed to dispatch_ai_command and not accidentally intercepted by the metacmd parser (which handles \ commands). If the REPL parser matches any /-prefixed token as a metacmd, /refresh could double-execute.

[STYLE / LOW] is_budget_exempt list in dispatch_ai_command will need updating for every new / command

The long || chain is already 20+ conditions. Consider inverting: define a requires_ai_budget function that returns true only for /ask, /fix, /explain, /optimize, /describe, and gate budget checking there. Everything else is exempt by default.

[DOCS / LOW] COMMANDS.md lists /n <name> [args...] but the /n+, /nd, /np commands use +, d, p suffixes — should be clear these are distinct commands

The table reads naturally but a new user might not realize /n+ is a standalone command, not /n with argument +. A footnote or note in the rationale section would help.

Potential issues (low confidence)

[BEHAVIOR] Deprecation messages go to stderr via eprintln!

All \cmd is deprecated; use /cmd instead. messages use eprintln!. This is correct for interactive use. However, if a user pipes rpg output or uses -c, the deprecation warning on stderr may intermix with query output in unexpected ways. This matches how psql handles warnings, so it is acceptable.

Summary

Good architectural change — the \ vs / namespace split is clear and the COMMANDS.md reference doc is useful. The only blocker is the rustfmt failure on the /ash handler call. Fix agent should run cargo fmt and push. Once CI is green, the PR is solid.

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 1.58228% with 311 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.68%. Comparing base (48b4ed2) to head (7c2c7f5).

@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
- Coverage   69.28%   68.68%   -0.61%     
==========================================
  Files          48       48              
  Lines       32216    32500     +284     
==========================================
  Hits        22320    22320              
- Misses       9896    10180     +284     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

REV Re-Review — PR #755: feat(repl): establish /cmd convention for rpg-specific commands

Branch: feat/754-slash-convention | CI: green (latest run: success at 02:41 UTC)
Prior blocker: rustfmt on /ash handler — resolved in the latest push (CI now clean).

BLOCKING

None.

NON-BLOCKING

[BUG / LOW] /n prefix ambiguity: /ns, /nd, /np, /n+ all match the /n handler

dispatch_ai_command matches /n commands in this order (bottom of the chain):

} else if let Some(rest) = input.strip_prefix("/n+") ...     // /n+
} else if let Some(rest) = input.strip_prefix("/nd ") ...    // /nd <name>
} else if let Some(rest) = input.strip_prefix("/np ") ...    // /np <name>
} else if let Some(rest) = input.strip_prefix("/ns ") ...    // /ns <name> <query>
} else if let Some(rest) = input.strip_prefix("/n").map(str::trim) ...   // /n <name>  ← catches /ns, /nd, /np without space

The /n catch-all fires for any input starting with /n that didn't match the earlier arms. This means /ns<enter> (no space, no args) falls into the /n arm and prints Usage: /n <name> [args...] instead of Usage: /ns <name> <query>. Same for /nd<enter> and /np<enter>. Low severity because the happy path (with a space) works correctly, and the error message is still user-visible. Fix: match /n (with trailing space or end-of-string check) last, or check that rest doesn't start with 's', 'd', 'p', '+'.

[BEHAVIOR / LOW] Deprecation warnings fire on every keystroke in readline pre-enter evaluation

The \refresh, \f2–\f5, \mode, etc. deprecation eprintln! calls fire inside dispatch_meta, which is called on every completed metacommand. This is correct and intentional. However, \log-file prints the deprecation notice before the path logic runs — so if the path is invalid, the user sees the deprecation warning followed by an error. Minor UX nit; not blocking.

[STYLE / LOW] is_budget_exempt list in dispatch_ai_command now includes all non-AI commands

The exemption list has grown to 20+ starts_with checks. This works correctly, but if a new / command is added without adding it here, token budget would incorrectly gate a non-AI command. Consider inverting the logic: only require the budget gate for the AI commands (/ask, /fix, /explain, /optimize, /describe, /compact) and let everything else through by default. This would make the exemption list unnecessary and reduce maintenance risk.

Summary

Verdict: APPROVE (no blockers). The rustfmt fix is clean; CI is green. Ready to merge.

CI is green and REV re-review finds no blocking issues (rustfmt fix confirmed). This PR is ready for merge — awaiting Nik's approval.

REV

PR #755 — feat(repl): establish /cmd convention for rpg-specific commands

This is a good structural decision — clean namespace separation between psql-compat (\) and rpg-specific (/). COMMANDS.md is a useful reference artifact.

Observations:

1. Budget-exempt list in dispatch_ai_command — the new path adds 15+ starts_with checks to a function called on every /-prefixed input. This is a maintenance trap: every new non-AI command needs to be added here or it'll incorrectly hit the budget gate. Consider inverting the logic: have a small allowlist of AI commands that do consume tokens (/ask, /fix, /explain, /optimize, /describe) rather than an ever-growing denylist of non-AI commands.

2. dispatch_ai_command return type change (-> Option<MetaResult>) — this is a real API change. The diff is truncated so I can't see the full signature, but callers of this function need to handle the new return value. If any call site ignores it (e.g. let _ = dispatch_ai_command(...)), the reconnect behavior from /session resume will silently fail.

3. Deprecation hint for \dba, \sql, etc. — COMMANDS.md documents these as deprecated with a notice. Make sure the runtime deprecation hint message is consistent with the table in COMMANDS.md (both say the same replacement).

4. COMMANDS.md is thorough and well-organized — the table format is readable and the ordering (AI first, then DBA, then modes) is sensible. The rationale section is worth keeping.

5. \/ash added to the rpg command list — this references pg_ash extension. The COMMANDS.md entry says "requires pg_ash extension" — confirm the runtime help text (/commands output) also includes this caveat.

The inverted allowlist (#1) and the Option<MetaResult> call-site audit (#2) are the main items to verify before merge.

Testing Evidence — feat/754-slash-convention

Tested on commit 6cefd64 (rpg 0.8.3), branch feat/754-slash-convention, against PostgreSQL 17.7.

Build:

$ cd /tmp/rpg-pr-755 && cargo build --jobs 1
   Compiling rpg v0.8.3 (/tmp/rpg-pr-755)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 28s

1. \ commands (psql-compatible) still work

ashtest=# \conninfo
You are connected to database "ashtest" as user "postgres" on host "127.0.0.1" at port "15433".
SSL connection (protocol: TLS, cipher: unknown, compression: off)

ashtest=# \l
                                                    List of databases
   Name    |  Owner   | Encoding | Locale Provider |  Collate   |   Ctype    | Locale | ICU Rules |   Access privileges
-----------+----------+----------+-----------------+------------+------------+--------+-----------+-----------------------
 ashtest   | postgres | UTF8     | libc            | en_US.utf8 | en_US.utf8 |        |           |
 demo_saas | postgres | UTF8     | libc            | en_US.utf8 | en_US.utf8 |        |           |
 postgres  | postgres | UTF8     | libc            | en_US.utf8 | en_US.utf8 |        |           |
 template0 | postgres | UTF8     | libc            | en_US.utf8 | en_US.utf8 |        |           | =c/postgres          +
 template1 | postgres | UTF8     | libc            | en_US.utf8 | en_US.utf8 |        |           | =c/postgres          +
(5 rows)

ashtest=# \dt
            List of relations
 Schema |     Name     | Type  |  Owner
--------+--------------+-------+----------
 public | big_workload | table | postgres
 public | workload     | table | postgres
(2 rows)

2. / commands (rpg-specific) work

ashtest=# /version
rpg 0.8.3 (6cefd64, built 2026-03-26)
Server: PostgreSQL 17.7 (Debian 17.7-3.pgdg13+1)

ashtest=# /mode
Input mode: sql  Execution mode: interactive

ashtest=# /dba
\dba diagnostic commands:
  \dba activity    pg_stat_activity: grouped by state, duration, wait events
  \dba locks       Lock tree (blocked/blocking)
  \dba waits       Wait event breakdown (+ for AI interpretation)
  \dba bloat       Table bloat estimates (dead tuples)
  \dba vacuum      Vacuum status and dead tuples
  \dba tablesize   Largest tables
  \dba connections Connection counts by state
  ...

ashtest=# /commands
No custom commands loaded.
Add Lua scripts to ~/.config/rpg/commands/*.lua

/dba activity hitting live pg_stat_activity:

ashtest=# /dba activity
 pid    | user     | db      | client        | state  | wait_type | wait_event    | backend_type   | duration | query
--------+----------+---------+---------------+--------+-----------+---------------+----------------+----------+-------
 729857 | postgres | ashtest | 172.17.0.1/32 | active | LWLock    | BufferMapping | client backend |       2s | SET enable_indexscan = off; SELECT count(payload) FROM big_workload WHERE id > 0
 729883 | postgres | ashtest | 172.17.0.1/32 | active |           |               | client backend |       1s | SELECT sum(i * i * i) FROM generate_series(1, 10000000) i;
 ...

3. Unknown /foo gives clear error listing all available / commands

ashtest=# /foo
Unknown command: /foo
AI: /ask, /fix, /explain, /optimize, /describe, /init, /clear, /compact, /budget
Diagnostics: /dba, /ash
Modes: /sql, /text2sql, /t2s, /plan, /yolo, /interactive, /mode
Queries: /ns, /n, /n+, /nd, /np
REPL: /profiles, /refresh, /session, /log-file, /explain-share, /commands, /version, /f2-f5

4. Deprecated \ aliases print a deprecation notice and still work

ashtest=# \dba
\dba is deprecated; use /dba instead.
\dba diagnostic commands:
  ...

ashtest=# \version
\version is deprecated; use /version instead.
rpg 0.8.3 (6cefd64, built 2026-03-26)
Server: PostgreSQL 17.7 (Debian 17.7-3.pgdg13+1)

ashtest=# \sql
\sql is deprecated; use /sql instead.
Input mode: sql

ashtest=# \mode
\mode is deprecated; use /mode instead.
Input mode: sql  Execution mode: interactive

5. \? shows the clear separation between \ and / namespaces

\? output (truncated — full output is 139 lines):

Backslash commands:
  \q              quit rpg
  \timing [on|off]      toggle/set query timing display
  \x [on|off|auto]      toggle/set expanded display
  \conninfo[+]    show connection information
  \?              show this help
  ...
  \d  [pattern]     describe objects
  \dt [pattern]     list tables
  \du [pattern]     list roles
  ...

AI commands:
  /ask <prompt>     natural language to SQL
  /explain          explain the last query plan
  /fix              diagnose and fix the last error
  /optimize <query> suggest query optimizations
  /init             generate .rpg.toml and POSTGRES.md in current directory
  /clear            clear AI conversation context
  /budget           show token usage and remaining budget

DBA diagnostics:
  /dba               show available diagnostics
  /dba activity      pg_stat_activity summary
  /dba bloat         table bloat estimates
  /dba locks         lock tree (blocked/blocking)
  /dba vacuum        vacuum status and dead tuples
  /ash               active session history shorthand
  ...

Named queries:
  /ns <name> <query>  save a named query
  /n  <name> [args…]  execute a named query
  /n+                 list all named queries
  /nd <name>          delete a named query
  /np <name>          print a named query without executing

Input/execution modes:
  /sql              switch to SQL input mode (default)
  /text2sql / /t2s  switch to text2sql input mode
  /plan             enter plan execution mode
  /yolo             YOLO mode: auto-enable text2sql, auto-execute
  /interactive      return to interactive mode (default)
  /mode             show current input and execution mode

REPL management:
  /profiles         list configured connection profiles
  /session list     show recent sessions
  /refresh          reload schema cache for tab completion
  /commands         list custom Lua meta-commands
  /version          show rpg version and build information

Deprecated (still work, prefer / equivalents above):
  \dba, \sql, \text2sql, \t2s, \mode, \plan, \yolo, \interactive
  \profiles, \refresh, \session, \log-file, \explain share
  \commands, \version, \f2, \f3, \f4, \f5
  \ns, \n, \n+, \nd, \np

Summary: Convention is working as designed. \ = psql-compatible, / = rpg-specific. Unknown / gives a helpful categorized list. Deprecated \ aliases still work with a clear migration message. \? renders the two namespaces in separate clearly-labeled sections with a Deprecated footer.

One minor note: the /dba and \dba help text internally still refers to \dba activity instead of /dba activity — may want to fix the sub-command labels in a follow-up.

REV Code Review — PR #755 feat(repl): establish /cmd convention for rpg-specific commands

Reviewed by: Max (automated REV review)
Branch: feat/754-slash-convention
Commit: 6cefd64

VERDICT: REQUEST_CHANGES

Two blocking bugs found. Both confirmed with live testing.

BLOCKING ISSUES (2)

CRITICAL src/repl/ai_commands.rs:429 — /explain-share is completely broken (dead code)

The /explain branch uses strip_prefix("/explain") and is checked at line 429, before the /explain-share handler at line 606. Since /explain-share depesz starts with /explain, it is consumed by the wrong handler — passing -share depesz as the query argument to handle_ai_explain.

Evidence:

ashtest=# /explain-share depesz
ERROR:  syntax error at or near "-"
LINE 1: -share depesz
                     ^

/explain-share is listed in \? help, listed in COMMANDS.md, listed in the unknown-command error message — but calling it causes a SQL error. Users will be confused and file bug reports.

Fix: Move the /explain-share check before the /explain check, or change /explain to use an exact-match + space check:

} else if input == "/explain" || input.starts_with("/explain ") {
    // ...
} else if let Some(service) = input.strip_prefix("/explain-share").map(str::trim) {

HIGH src/repl/ai_commands.rs:648,688,708,720 — /n* prefix collision causes silent wrong dispatch

The dispatch chain for named queries uses raw strip_prefix without requiring a space or end-of-input boundary. This causes:

• /ndrop → dispatched to /nd with name "rop" instead of "unknown command"
• /nstuff → dispatched to /ns with rest="tuff", prints usage error for /ns
• /nprod → dispatched to /np with name="rod"

Evidence:

ashtest=# /ndrop
/nd: unknown query "rop"

ashtest=# /nstuff
Usage: /ns <name> <query>

A user with named query drop_old_data who accidentally types /ndrop_old_data (no space) gets routed to /nd (delete!). While named query names must be alphanumeric+underscore so - is not valid, this is still confusing and error-prone.

Fix: Require a space or end-of-input after the command prefix:

} else if input == "/ns" || input.starts_with("/ns ") {
    let rest = input[3..].trim();
    // ...
} else if input == "/nd" || input.starts_with("/nd ") {
    let name = input[3..].trim();
    // ...
} else if input == "/np" || input.starts_with("/np ") {
    let name = input[3..].trim();
    // ...
} else if input == "/n+" {
    // ...
} else if input == "/n" || input.starts_with("/n ") {
    let rest = input[2..].trim();
    // ...

NON-BLOCKING ISSUES (3)

MEDIUM README.md — Not updated: still uses deprecated \ notation throughout

The README has 25+ references to rpg-specific commands using the old \ notation. This PR establishes the \ vs / convention but leaves the main user-facing documentation completely out of date.

Specific lines with old notation (non-exhaustive):

• Line 15: \dba commands
• Lines 82–121: \text2sql, \yolo, \sql, \interactive, \t2s
• Lines 272, 278–301: \commands, \dba activity, \dba locks, etc.

The PR adds COMMANDS.md (good) and updates CLAUDE.md (good), but users reading README.md will see the old commands and be confused when \dba triggers a deprecation warning.

Suggestion: Update README.md to use /dba, /yolo, /sql, /text2sql, /commands etc. throughout. The deprecation notice in the binary is not sufficient — the docs must lead the way.

MEDIUM src/repl/ai_commands.rs — Budget-exempt list uses inconsistent matching (missing bare command forms)

The is_budget_exempt check uses starts_with("/ns ") (with trailing space) but the actual dispatch uses strip_prefix("/ns") (no space required). This means bare /ns (no arguments) is NOT budget-exempt but WILL be dispatched to the /ns handler (which prints a usage message). If the budget is exhausted, /ns silently returns None instead of printing the usage hint.

Same issue for /nd, /np.

|| input.starts_with("/ns ")   // misses bare "/ns"
|| input == "/n+"
|| input.starts_with("/nd ")   // misses bare "/nd"  
|| input.starts_with("/np ")   // misses bare "/np"

Suggestion: Use input == "/ns" || input.starts_with("/ns ") pattern throughout the exempt list, or simplify to input.starts_with("/ns").

LOW src/repl/mod.rs (dba module) — /dba help output still shows \dba notation

When you run /dba or \dba, the output lists subcommands using the old \dba prefix:

ashtest=# /dba
\dba diagnostic commands:
  \dba activity    pg_stat_activity: grouped by state, duration, wait events
  \dba locks       Lock tree (blocked/blocking)
  \dba io          I/O statistics by backend type (PG 16+, verbose: \dba+ io)
  ...

The \? help correctly shows /dba activity etc., but the dba module's own help text was not updated. This creates an inconsistency: \? says /dba activity, but running /dba tells you to use \dba activity.

The fix is in the dba module (not in the diff — was missed). Update the dba help text to use /dba prefix.

LOW CHANGELOG.md — No entry for this PR

This is a user-visible behavior change (deprecation warnings, new namespace convention, new /commands, /version, /mode as primary commands). Missing from CHANGELOG.

TEST RESULTS

Test environment: PostgreSQL 17.7, rpg 0.8.3 (commit 6cefd64)

A. Backslash commands (psql-compatible) — PASS ✓

ashtest=# \d
             List of relations
 Schema |      Name      | Type  |  Owner
--------+----------------+-------+----------
 public | big_workload   | table | postgres
 public | pg_buffercache | view  | postgres
 public | workload       | table | postgres
 public | workload_pkey  | index | postgres
(4 rows)

ashtest=# \dt
            List of relations
 Schema |     Name     | Type  |  Owner
--------+--------------+-------+----------
 public | big_workload | table | postgres
 public | workload     | table | postgres
(2 rows)

ashtest=# \conninfo
You are connected to database "ashtest" as user "postgres" on host "127.0.0.1" at port "15433".
SSL connection (protocol: TLS, cipher: unknown, compression: off)

ashtest=# \timing
Timing is on.
ashtest=# \timing
Timing is off.

\l also works (lists 5 databases correctly). All psql-compatible commands unaffected.

B. Slash commands (rpg-specific) — PARTIAL PASS

ashtest=# /version
rpg 0.8.3 (6cefd64, built 2026-03-26)
Server: PostgreSQL 17.7 (Debian 17.7-3.pgdg13+1)

ashtest=# /mode
Input mode: sql  Execution mode: interactive

ashtest=# /commands
No custom commands loaded.
Add Lua scripts to ~/.config/rpg/commands/*.lua

ashtest=# /dba
\dba diagnostic commands:
  \dba activity    pg_stat_activity: grouped by state, duration, wait events
  \dba locks       Lock tree (blocked/blocking)
  ...

/version, /mode, /commands work correctly. /dba works but displays old \dba notation in its help text.

C. Help output (\?) — PASS with minor inconsistency

Full \? output (captured via rpg -c '\?'):

rpg 0.8.3 (6cefd64, built 2026-03-26)

Backslash commands:
  \q              quit rpg
  \timing [on|off]      toggle/set query timing display
  ...

AI commands:
  /ask <prompt>     natural language to SQL
  /explain          explain the last query plan
  /fix              diagnose and fix the last error
  ...

DBA diagnostics:
  /dba               show available diagnostics
  /dba activity      pg_stat_activity summary
  /dba ash           active session history (requires pg_ash)
  ...

Named queries:
  /ns <name> <query>  save a named query (name: alphanumerics + underscores)
  /n  <name> [args…]  execute a named query; $1,$2,… replaced by args
  /n+                 list all named queries with their SQL
  /nd <name>          delete a named query
  /np <name>          print a named query without executing

Input/execution modes:
  /sql              switch to SQL input mode (default)
  /text2sql / /t2s  switch to text2sql input mode
  /plan             enter plan execution mode
  /yolo             YOLO mode: auto-enable text2sql, hide SQL box, auto-execute
  /interactive      return to interactive mode (default)
  /mode             show current input and execution mode

EXPLAIN sharing:
  /explain-share depesz    upload last EXPLAIN plan to explain.depesz.com
  /explain-share dalibo    upload last EXPLAIN plan to explain.dalibo.com
  /explain-share pgmustard upload last EXPLAIN plan to app.pgmustard.com

REPL management:
  /profiles         list configured connection profiles
  /session list     show recent sessions
  /session save     save the current session
  /session delete <id>   delete a session
  /session resume <id>   reconnect using a saved session
  /refresh          reload schema cache for tab completion
  /log-file <path>  start logging queries to path (no arg = stop)
  /commands         list custom Lua meta-commands (if any are configured)
  /version          show rpg version and build information

Function keys (interactive mode):
  F2 / /f2       toggle schema-aware tab completion on/off
  F3 / /f3       toggle single-line mode on/off
  F4 / /f4       toggle Vi/Emacs editing mode (next session)
  F5 / /f5       toggle auto-EXPLAIN on/off
  Ctrl-T          toggle SQL/text2sql input mode

Deprecated (still work, prefer / equivalents above):
  \dba, \sql, \text2sql, \t2s, \mode, \plan, \yolo, \interactive
  \profiles, \refresh, \session, \log-file, \explain share
  \commands, \version, \f2, \f3, \f4, \f5
  \ns, \n, \n+, \nd, \np

The \? output correctly uses / prefix for all rpg-specific commands and lists deprecated \ aliases at the bottom. Structure is clear.

One inconsistency: \? shows /explain-share but the command is broken (see blocking issue #1).

D. Unknown command handling — PASS ✓

ashtest=# /foo
Unknown command: /foo
AI: /ask, /fix, /explain, /optimize, /describe, /init, /clear, /compact, /budget
Diagnostics: /dba, /ash
Modes: /sql, /text2sql, /t2s, /plan, /yolo, /interactive, /mode
Queries: /ns, /n, /n+, /nd, /np
REPL: /profiles, /refresh, /session, /log-file, /explain-share, /commands, /version, /f2-f5

ashtest=# /bar
Unknown command: /bar
AI: /ask, /fix, /explain, /optimize, /describe, /init, /clear, /compact, /budget
...

Clear error message with full list of available commands.

E. Deprecated \ aliases — PASS ✓

ashtest=# \dba
\dba is deprecated; use /dba instead.
\dba diagnostic commands:
  \dba activity    pg_stat_activity: grouped by state, duration, wait events
  ...

ashtest=# \version
\version is deprecated; use /version instead.
rpg 0.8.3 (6cefd64, built 2026-03-26)
Server: PostgreSQL 17.7 (Debian 17.7-3.pgdg13+1)

ashtest=# \mode
\mode is deprecated; use /mode instead.
Input mode: sql  Execution mode: interactive

Deprecation notices print correctly before executing the command.

F. Edge cases — MIXED

ashtest=# /
Unknown command: /
AI: /ask, /fix, ...

ashtest=# /   
Unknown command: /
AI: /ask, /fix, ...

ashtest=# /COMMANDS
Unknown command: /COMMANDS
AI: /ask, /fix, ...

Bare / and / with spaces: handled gracefully (unknown command message). /COMMANDS uppercase: treated as unknown — commands are case-sensitive (by design, but undocumented).

Prefix collision bugs:

ashtest=# /ndrop
/nd: unknown query "rop"

ashtest=# /nstuff
Usage: /ns <name> <query>

These confirm the blocking issue #2.

G. README/docs assessment — FAIL

The README has not been updated and still uses the old \ notation throughout. Key examples:

• Line 15: **DBA diagnostics** — 15+ \\dba` commands for activity, locks, bloat, indexes`
• Lines 82–121: Entire \text2sql and \yolo sections use old notation
• Lines 278–301: DBA diagnostics section uses \dba activity, \dba locks etc.

COMMANDS.md is a good addition (comprehensive, clear) but user-facing docs in README.md must be updated before this PR lands. The convention is only useful if users can discover it from the docs, not just from \? or COMMANDS.md.

H. \? help assessment — PASS ✓ (modulo the broken /explain-share)

All major / commands appear in \?:

• ✓ /ask, /explain, /fix, /optimize, /describe, /init, /clear, /compact, /budget
• ✓ /dba, /ash (with subcommands listed)
• ✓ /ns, /n, /n+, /nd, /np
• ✓ /sql, /text2sql, /t2s, /plan, /yolo, /interactive, /mode
• ✓ /profiles, /refresh, /session, /log-file, /explain-share, /commands, /version
• ✓ /f2, /f3, /f4, /f5

Section headers are clear (AI commands, DBA diagnostics, Named queries, Input/execution modes, REPL management). Deprecated aliases listed at bottom. Good structure overall.

Summary

What's good

The overall direction is right. The \ vs / convention is clean and well-reasoned. COMMANDS.md is comprehensive. The \? help output structure is excellent — clear sections, deprecated aliases listed separately. Deprecation notices in the binary are well done. The dispatch_ai_command return type change from () to Option<MetaResult> correctly enables /session resume to propagate reconnection.

REV-assisted review (AI analysis by postgres-ai/rev)

REV Re-Review — PR #755 (after fixes)

Branch: feat/754-slash-convention
Head: 7312985
Commits since REQUEST_CHANGES: 5694b29, 7312985

BLOCKING ISSUES

None. Both blocking bugs from the prior review are resolved.

Fix verification

B1 — /explain-share dead code (RESOLVED)

/explain-share handler is now at line 429, before the /explain handler at line 436. The starts_with("/explain") match no longer swallows /explain-share depesz. Fix is correct.

B2 — /n* prefix collision (RESOLVED)

All four dispatch branches now use the input == "/ns" || input.starts_with("/ns ") pattern:

• /ns — line 647
• /nd — line 688
• /np — line 709
• /n — line 722

/ndrop will no longer silently route to /nd. Fix is correct.

README update (requested as NON-BLOCKING — done)

All \dba, \text2sql, \yolo, \sql, \interactive, \commands, \explain share references updated to /dba, /text2sql, /yolo, /sql, /interactive, /commands, /explain-share. The 25+ stale references are addressed.

NON-BLOCKING observations (new diff only)

LOW — /explain-share bare call prints usage, not the more helpful "no plan to share" message

When called as /explain-share (no service arg), the handler prints Usage: /explain-share <depesz|dalibo|pgmustard>. That's fine. But the prior behavior also handled the empty-service case via dispatch_explain_share, which would check settings.last_explain_text first and print a more contextual error. Current behavior is acceptable — minor UX detail, not blocking.

INFO — /explain-share still accepts partial prefix match

/explain-shareXYZ (no space, garbage suffix) would still match strip_prefix("/explain-share") and pass "XYZ" as the service. The service matching inside dispatch_explain_share will reject it with an unknown-service error. Not a bug, just noting the behavior is consistent with the rest of the dispatch chain.

Summary

CI is green. No blocking findings. PR #755 is ready for merge — awaiting Nik's approval.

Testing evidence — bug fixes (commit 7312985)

Branch feat/754-slash-convention HEAD: 7312985
Build: cargo build clean, 0 errors.

Fix 1: /explain-share dead code (commit 5694b29)

Root cause: strip_prefix("/explain") at line ~429 consumed /explain-share before its own branch could fire.

Fix: /explain-share branch moved before /explain in dispatch_ai_command:

// src/repl/ai_commands.rs
// /explain-share <service> — upload last EXPLAIN plan to external visualiser.
} else if let Some(service) = input.strip_prefix("/explain-share").map(str::trim) {   // line 430 — now BEFORE /explain
    if service.is_empty() {
        eprintln!("Usage: /explain-share <depesz|dalibo|pgmustard>");
        return Some(MetaResult::Done);
    }
    dispatch_explain_share(client, settings, service).await;
...
} else if let Some(query_arg) = input.strip_prefix("/explain").map(str::trim) {       // line ~450 — after /explain-share

Verified: grep -n 'strip_prefix.*explain' src/repl/ai_commands.rs

429:    } else if let Some(service) = input.strip_prefix("/explain-share").map(str::trim) {
450:    } else if let Some(query_arg) = input.strip_prefix("/explain").map(str::trim) {

/explain-share is at line 429, /explain at line 450 — ordering is correct. ✅

Fix 2: /n* prefix collisions (commit 5694b29)

Root cause: strip_prefix("/nd") matched /ndrop, routing it to the delete handler with name = "rop".

Fix: All short-prefix /n branches now require exact match or trailing space:

} else if input == "/ns" || input.starts_with("/ns ") {
    let rest = input["/ns".len()..].trim();
} else if input == "/nd" || input.starts_with("/nd ") {
    let name = input["/nd".len()..].trim();
} else if input == "/np" || input.starts_with("/np ") {
    let name = input["/np".len()..].trim();
} else if input == "/n" || input.starts_with("/n ") {
    let rest = input["/n".len()..].trim();

Verified: grep -n 'input == .*/n\|starts_with.*"/n' src/repl/ai_commands.rs

648:    } else if input == "/ns" || input.starts_with("/ns ") {
658:    } else if input == "/nd" || input.starts_with("/nd ") {
672:    } else if input == "/np" || input.starts_with("/np ") {
685:    } else if input == "/n" || input.starts_with("/n ") {

/ndrop no longer matches /nd — falls through to unknown command handler. ✅

Note on interactive testing

/ commands are only dispatched in interactive (TTY) REPL mode via dispatch_ai_command. The -c flag and piped stdin bypass this path entirely (by design, matching psql behavior). Code-level verification above confirms the routing is correct. The prior testing evidence on commit 6cefd64 covered the interactive session format — the fix commits are surgical changes to the dispatch chain only.

CI is green and REV re-review confirms both blocking bugs resolved (B1: /explain-share dead code fixed, B2: /n* prefix collision fixed). Testing evidence posted. This PR is ready for merge — awaiting Nik's approval.