diff --git a/skill/SKILL.md b/skill/SKILL.md index 6968d67..5ea2d76 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -28,7 +28,9 @@ can't hang forever, and output is dense and machine-readable. `--statement-timeout`, `--lock-timeout`). A bad query fails fast instead of pinning a backend. - **Structured output.** Add `--json` to query/diagnostic commands when the - result feeds further work. Human tables otherwise. + result feeds further work. Human tables otherwise — and those auto-densify + when output is piped or captured (no box-drawing or padding, same info, fewer + tokens); pass `--pretty` to force the decorated form. - **Semantic exit codes** — branch on them, don't parse text: - `0` healthy / success - `1` warning (non-critical finding) diff --git a/src/commands/capabilities.rs b/src/commands/capabilities.rs index 3f1e477..d629d3b 100644 --- a/src/commands/capabilities.rs +++ b/src/commands/capabilities.rs @@ -689,8 +689,49 @@ fn check_fix_terminate_capability(has_pg_terminate: bool, read_only: bool) -> Ca } } -/// Print capabilities in human-readable format -pub fn print_human(result: &CapabilitiesResult) { +/// Print capabilities in human-readable format. +/// +/// `Pretty` pads the capability id to a fixed column and uses glyph status +/// markers. `Dense` drops the padding and glyphs (status as a word) — same +/// per-capability id, status, and notes, fewer tokens. +pub fn print_human(result: &CapabilitiesResult, density: crate::output::Density) { + if density.is_dense() { + print_dense(result); + } else { + print_pretty(result); + } +} + +/// Plain status word, shared so both forms agree on the vocabulary. +fn status_word(status: CapabilityStatus) -> &'static str { + match status { + CapabilityStatus::Available => "available", + CapabilityStatus::Degraded => "degraded", + CapabilityStatus::Unavailable => "unavailable", + CapabilityStatus::Unknown => "unknown", + } +} + +fn print_dense(result: &CapabilitiesResult) { + println!("CAPABILITIES:"); + for cap in &result.capabilities { + println!("{} {}", cap.id, status_word(cap.status)); + for lim in &cap.limitations { + println!(" - {}", lim); + } + if cap.status != CapabilityStatus::Available { + for reason in &cap.reasons { + println!(" - {}", reason.message); + } + } + } + println!( + "SUMMARY: {} available, {} degraded, {} unavailable", + result.summary.available, result.summary.degraded, result.summary.unavailable + ); +} + +fn print_pretty(result: &CapabilitiesResult) { println!("CAPABILITIES:"); println!(); diff --git a/src/commands/context.rs b/src/commands/context.rs index 8bf8fda..35eb131 100644 --- a/src/commands/context.rs +++ b/src/commands/context.rs @@ -281,8 +281,90 @@ pub async fn run_context( }) } -/// Print context in human-readable format -pub fn print_human(result: &ContextResult) { +/// Print context in human-readable format. +/// +/// `Pretty` keeps the padded, blank-line-separated layout a person reads. +/// `Dense` strips the blank lines and padding and folds extensions/roles onto +/// single lines — same fields, far fewer tokens for an agent capturing it. +pub fn print_human(result: &ContextResult, density: crate::output::Density) { + if density.is_dense() { + print_dense(result); + } else { + print_pretty(result); + } +} + +fn yes_no(b: bool) -> &'static str { + if b { + "yes" + } else { + "no" + } +} + +fn print_dense(result: &ContextResult) { + let ctx = &result.context; + + println!( + "CONNECTION: {}@{}:{}/{} ({})", + ctx.target.user, + ctx.target.host, + ctx.target.port, + ctx.target.database, + if ctx.target.readonly { + "read-only" + } else { + "read-write" + } + ); + + let recovery = if ctx.server.in_recovery { + "replica" + } else { + "primary" + }; + match ctx.server.data_directory { + Some(ref dir) => println!( + "SERVER: pg {} ({}) {} data_dir={}", + ctx.server.version_major, ctx.server.version_num, recovery, dir + ), + None => println!( + "SERVER: pg {} ({}) {}", + ctx.server.version_major, ctx.server.version_num, recovery + ), + } + + let mut exts: Vec<_> = ctx.extensions.iter().collect(); + exts.sort_by_key(|(name, _)| name.as_str()); + let ext_list: Vec = exts + .iter() + .map(|(name, version)| format!("{}={}", name, version)) + .collect(); + if ext_list.is_empty() { + println!("EXTENSIONS (0):"); + } else { + println!( + "EXTENSIONS ({}): {}", + ctx.extensions.len(), + ext_list.join(" ") + ); + } + + println!( + "PRIVILEGES: superuser={} pg_stat_activity={} pg_cancel_backend={} pg_terminate={} pg_stat_statements={}", + yes_no(ctx.privileges.is_superuser), + yes_no(ctx.privileges.pg_stat_activity_select), + yes_no(ctx.privileges.pg_cancel_backend_execute), + yes_no(ctx.privileges.pg_terminate_backend_execute), + yes_no(ctx.privileges.pg_stat_statements_select), + ); + + if !ctx.privileges.roles.is_empty() { + println!("ROLES: {}", ctx.privileges.roles.join(" ")); + } +} + +fn print_pretty(result: &ContextResult) { let ctx = &result.context; println!("CONNECTION:"); diff --git a/src/commands/schema.rs b/src/commands/schema.rs index a996135..ccc5d24 100644 --- a/src/commands/schema.rs +++ b/src/commands/schema.rs @@ -694,35 +694,54 @@ pub async fn describe( return Ok(()); } - // Human mode: formatted output + // Human mode: formatted output. Dense drops the decorative rule and the + // blank lines framing each section; pretty keeps them. + let density = output.density(); + let dense = density.is_dense(); let mut result = String::new(); - result.push('\n'); + if !dense { + result.push('\n'); + } result.push_str(&format!( "Table: {}.{}\n", quote_ident(&resolved.schema), quote_ident(&resolved.name) )); - result.push_str(&"─".repeat(64)); - result.push('\n'); - result.push('\n'); - result.push_str(&table_info.format(verbose)); + if !dense { + result.push_str(&"─".repeat(64)); + result.push('\n'); + result.push('\n'); + } + result.push_str(&table_info.format(verbose, density)); // Append dependents/dependencies section if requested if let Some(ref deps) = deps_data { - result.push('\n'); - result.push('\n'); + if dense { + result.push('\n'); + } else { + result.push('\n'); + result.push('\n'); + } result.push_str("Direct Dependents:"); result.push('\n'); - result.push('\n'); - result.push_str(&deps.format(&resolved.schema, &resolved.name)); + if !dense { + result.push('\n'); + } + result.push_str(&deps.format(&resolved.schema, &resolved.name, density)); } if let Some(ref deps) = dependencies_data { - result.push('\n'); - result.push('\n'); + if dense { + result.push('\n'); + } else { + result.push('\n'); + result.push('\n'); + } result.push_str("Direct Dependencies:"); result.push('\n'); - result.push('\n'); - result.push_str(&deps.format(&resolved.schema, &resolved.name)); + if !dense { + result.push('\n'); + } + result.push_str(&deps.format(&resolved.schema, &resolved.name, density)); } output.data(&result); diff --git a/src/commands/sql_cmd.rs b/src/commands/sql_cmd.rs index b38c38b..d6cdeb9 100644 --- a/src/commands/sql_cmd.rs +++ b/src/commands/sql_cmd.rs @@ -36,6 +36,8 @@ pub struct SqlOptions { pub timeouts: TimeoutConfig, pub quiet: bool, pub json: bool, + /// Readable-output density for result tables (ignored in JSON/quiet mode). + pub density: crate::output::Density, } #[derive(Serialize)] @@ -219,7 +221,7 @@ async fn run_read(client: &Client, sql: &str, opts: &SqlOptions) -> Result<()> { if opts.quiet { return Ok(()); } - print_results(&results); + print_results(&results, opts.density); Ok(()) } @@ -420,7 +422,7 @@ fn finish_write( } // Print any sample rows first, then the summary banner. - print_results(&results); + print_results(&results, opts.density); if committed { println!("\nCOMMITTED — {rows_affected} row(s) affected."); @@ -499,7 +501,7 @@ fn emit_json(opts: &SqlOptions, write: Option, results: Vec { - print_table(columns, rows); + print_table(columns, rows, density); if *truncated > 0 { println!("… +{truncated} more row(s) — use --limit N (or --limit 0 to uncap)"); } } SqlResult::Sample { columns, rows } => { println!("Sample of affected rows:"); - print_table(columns, rows); + print_table(columns, rows, density); } SqlResult::CommandComplete { rows } => { println!("OK ({rows} rows)"); @@ -656,11 +658,27 @@ fn classify(sql: &str) -> Result { }) } -fn print_table(columns: &[String], rows: &[Vec>]) { +fn print_table(columns: &[String], rows: &[Vec>], density: crate::output::Density) { if columns.is_empty() { return; } + if density.is_dense() { + // Dense: header row plus unpadded ` | `-joined cells, no width padding + // and no separator rule. Columns are still labelled and aligned by the + // pipe delimiter, so the result stays unambiguous. + println!("{}", columns.join(" | ")); + for row in rows { + let line: Vec<&str> = columns + .iter() + .enumerate() + .map(|(i, _)| row.get(i).and_then(|v| v.as_deref()).unwrap_or("NULL")) + .collect(); + println!("{}", line.join(" | ")); + } + return; + } + let mut widths: Vec = columns.iter().map(|c| c.len()).collect(); for row in rows { for (i, cell) in row.iter().enumerate() { diff --git a/src/commands/triage.rs b/src/commands/triage.rs index e61bcd1..7e13d59 100644 --- a/src/commands/triage.rs +++ b/src/commands/triage.rs @@ -687,18 +687,43 @@ async fn check_stats_age(client: &Client) -> CheckOutcome { } } -/// Print triage results in human-readable format -pub fn print_human(results: &TriageResults, quiet: bool) { +/// Plain status word for a check, shared across pretty and dense forms. +fn status_word(status: CheckStatus) -> &'static str { + match status { + CheckStatus::Healthy => "healthy", + CheckStatus::Warning => "WARNING", + CheckStatus::Critical => "CRITICAL", + } +} + +/// Print triage results in human-readable format. +/// +/// `Pretty` aligns labels and summaries into fixed-width columns and prefixes +/// each row with a status emoji. `Dense` drops the column padding and the +/// emoji (the status word carries the same signal) — this is the largest +/// per-token win, since the pretty table is mostly padding whitespace. +pub fn print_human(results: &TriageResults, quiet: bool, density: crate::output::Density) { if quiet { - // In quiet mode, only show non-healthy checks and skipped + // In quiet mode, only show non-healthy checks and skipped. Quiet output + // is already minimal; dense just drops the leading emoji. + let dense = density.is_dense(); for check in &results.checks { if check.status != CheckStatus::Healthy { - println!( - "{} {}: {}", - check.status.emoji(), - check.label, - check.summary - ); + if dense { + println!( + "{} {}: {}", + status_word(check.status), + check.label, + check.summary + ); + } else { + println!( + "{} {}: {}", + check.status.emoji(), + check.label, + check.summary + ); + } for action in &check.next_actions { println!(" → {}", action.to_command_string()); } @@ -714,6 +739,54 @@ pub fn print_human(results: &TriageResults, quiet: bool) { return; } + if density.is_dense() { + print_dense(results); + } else { + print_pretty(results); + } +} + +fn print_dense(results: &TriageResults) { + // One check per line: "LABEL: summary — status". No column padding, no + // emoji. Severity ordering is preserved from `results.checks`. + for check in &results.checks { + println!( + "{}: {} — {}", + check.label, + check.summary, + status_word(check.status) + ); + } + + if !results.skipped_checks.is_empty() { + println!("SKIPPED:"); + for skip in &results.skipped_checks { + println!(" {} - {}", skip.check_id, skip.reason_code.description()); + } + } + + let actionable: Vec<_> = results + .checks + .iter() + .filter(|c| !c.next_actions.is_empty() && c.status != CheckStatus::Healthy) + .collect(); + + if !actionable.is_empty() { + println!("NEXT ACTIONS:"); + for check in actionable { + for action in &check.next_actions { + println!( + " {} → {} ({})", + check.label, + action.to_command_string(), + action.rationale + ); + } + } + } +} + +fn print_pretty(results: &TriageResults) { // Find longest label for alignment let max_label = results .checks @@ -724,11 +797,7 @@ pub fn print_human(results: &TriageResults, quiet: bool) { // Print checks (already sorted by severity) for check in &results.checks { - let status_str = match check.status { - CheckStatus::Healthy => format!("{} healthy", check.status.emoji()), - CheckStatus::Warning => format!("{} WARNING", check.status.emoji()), - CheckStatus::Critical => format!("{} CRITICAL", check.status.emoji()), - }; + let status_str = format!("{} {}", check.status.emoji(), status_word(check.status)); println!( "{:width$} {:40} {}", diff --git a/src/describe.rs b/src/describe.rs index 58510d0..6084547 100644 --- a/src/describe.rs +++ b/src/describe.rs @@ -1040,69 +1040,100 @@ async fn get_column_types(client: &Client, schema: &str, table: &str) -> Result< } impl Dependents { - /// Format dependents output for display - pub fn format(&self, schema: &str, table: &str) -> String { + /// Format dependents output for display. + /// + /// `Dense` drops blank-line separators between the FK/Views/Triggers + /// subsections and omits any subsection that has no entries; `Pretty` keeps + /// every labelled subsection (with `(none)`) and the blank-line spacing. + pub fn format(&self, schema: &str, table: &str, density: crate::output::Density) -> String { + let dense = density.is_dense(); let mut output = Vec::new(); let mut count = 0; + let mut first = true; + let mut sep = |out: &mut Vec, present: bool| { + // In dense mode a subsection is only emitted when present, so we add + // no blank line; in pretty mode every subsection (after the first) + // is preceded by a blank line. + if !dense && !first { + out.push(String::new()); + } + if present || !dense { + first = false; + } + }; // Foreign Keys section - output.push(" Foreign Keys (tables referencing this table):".to_string()); - if self.foreign_keys.is_empty() { - output.push(" (none)".to_string()); - } else { - for fk in &self.foreign_keys { - let from_cols = if fk.from_columns.len() == 1 { - fk.from_columns[0].clone() - } else { - format!("({})", fk.from_columns.join(", ")) - }; - let to_cols = if fk.to_columns.len() == 1 { - fk.to_columns[0].clone() - } else { - format!("({})", fk.to_columns.join(", ")) - }; - output.push(format!( - " {}.{}.{} \u{2192} {}.{}.{}", - fk.from_schema, fk.from_table, from_cols, fk.to_schema, fk.to_table, to_cols - )); - count += 1; + if !dense || !self.foreign_keys.is_empty() { + sep(&mut output, !self.foreign_keys.is_empty()); + output.push(" Foreign Keys (tables referencing this table):".to_string()); + if self.foreign_keys.is_empty() { + output.push(" (none)".to_string()); + } else { + for fk in &self.foreign_keys { + let from_cols = if fk.from_columns.len() == 1 { + fk.from_columns[0].clone() + } else { + format!("({})", fk.from_columns.join(", ")) + }; + let to_cols = if fk.to_columns.len() == 1 { + fk.to_columns[0].clone() + } else { + format!("({})", fk.to_columns.join(", ")) + }; + output.push(format!( + " {}.{}.{} \u{2192} {}.{}.{}", + fk.from_schema, + fk.from_table, + from_cols, + fk.to_schema, + fk.to_table, + to_cols + )); + count += 1; + } } } // Views section - output.push(String::new()); - output.push(" Views:".to_string()); - if self.views.is_empty() { - output.push(" (none)".to_string()); - } else { - for view in &self.views { - let suffix = if view.is_materialized { - " (materialized)" - } else { - "" - }; - output.push(format!(" {}.{}{}", view.schema, view.name, suffix)); - count += 1; + if !dense || !self.views.is_empty() { + sep(&mut output, !self.views.is_empty()); + output.push(" Views:".to_string()); + if self.views.is_empty() { + output.push(" (none)".to_string()); + } else { + for view in &self.views { + let suffix = if view.is_materialized { + " (materialized)" + } else { + "" + }; + output.push(format!(" {}.{}{}", view.schema, view.name, suffix)); + count += 1; + } } } // Triggers section - output.push(String::new()); - output.push(" Triggers:".to_string()); - if self.triggers.is_empty() { - output.push(" (none)".to_string()); - } else { - for trg in &self.triggers { - output.push(format!( - " {} (on {}.{})", - trg.trigger_name, trg.schema, trg.table_name - )); - count += 1; + if !dense || !self.triggers.is_empty() { + sep(&mut output, !self.triggers.is_empty()); + output.push(" Triggers:".to_string()); + if self.triggers.is_empty() { + output.push(" (none)".to_string()); + } else { + for trg in &self.triggers { + output.push(format!( + " {} (on {}.{})", + trg.trigger_name, trg.schema, trg.table_name + )); + count += 1; + } } } // Summary - output.push(String::new()); + if !dense { + output.push(String::new()); + } output.push(format!("{} objects depend on {}.{}", count, schema, table)); output.join("\n") @@ -1110,64 +1141,89 @@ impl Dependents { } impl Dependencies { - /// Format dependencies output for display - pub fn format(&self, schema: &str, table: &str) -> String { + /// Format dependencies output for display. Density behaves as in + /// [`Dependents::format`]. + pub fn format(&self, schema: &str, table: &str, density: crate::output::Density) -> String { + let dense = density.is_dense(); let mut output = Vec::new(); let mut count = 0; + let mut first = true; + let mut sep = |out: &mut Vec, present: bool| { + if !dense && !first { + out.push(String::new()); + } + if present || !dense { + first = false; + } + }; // Foreign Keys section (this table references) - output.push(" Foreign Keys (this table references):".to_string()); - if self.foreign_keys.is_empty() { - output.push(" (none)".to_string()); - } else { - for fk in &self.foreign_keys { - let from_cols = if fk.from_columns.len() == 1 { - fk.from_columns[0].clone() - } else { - format!("({})", fk.from_columns.join(", ")) - }; - let to_cols = if fk.to_columns.len() == 1 { - fk.to_columns[0].clone() - } else { - format!("({})", fk.to_columns.join(", ")) - }; - output.push(format!( - " {}.{}.{} \u{2192} {}.{}.{}", - fk.from_schema, fk.from_table, from_cols, fk.to_schema, fk.to_table, to_cols - )); - count += 1; + if !dense || !self.foreign_keys.is_empty() { + sep(&mut output, !self.foreign_keys.is_empty()); + output.push(" Foreign Keys (this table references):".to_string()); + if self.foreign_keys.is_empty() { + output.push(" (none)".to_string()); + } else { + for fk in &self.foreign_keys { + let from_cols = if fk.from_columns.len() == 1 { + fk.from_columns[0].clone() + } else { + format!("({})", fk.from_columns.join(", ")) + }; + let to_cols = if fk.to_columns.len() == 1 { + fk.to_columns[0].clone() + } else { + format!("({})", fk.to_columns.join(", ")) + }; + output.push(format!( + " {}.{}.{} \u{2192} {}.{}.{}", + fk.from_schema, + fk.from_table, + from_cols, + fk.to_schema, + fk.to_table, + to_cols + )); + count += 1; + } } } // Trigger Functions section - output.push(String::new()); - output.push(" Triggers (functions called):".to_string()); - if self.trigger_functions.is_empty() { - output.push(" (none)".to_string()); - } else { - for tf in &self.trigger_functions { - output.push(format!( - " {}.{}() via {} trigger", - tf.function_schema, tf.function_name, tf.trigger_name - )); - count += 1; + if !dense || !self.trigger_functions.is_empty() { + sep(&mut output, !self.trigger_functions.is_empty()); + output.push(" Triggers (functions called):".to_string()); + if self.trigger_functions.is_empty() { + output.push(" (none)".to_string()); + } else { + for tf in &self.trigger_functions { + output.push(format!( + " {}.{}() via {} trigger", + tf.function_schema, tf.function_name, tf.trigger_name + )); + count += 1; + } } } // Types section - output.push(String::new()); - output.push(" Types:".to_string()); - if self.types.is_empty() { - output.push(" (none)".to_string()); - } else { - for t in &self.types { - output.push(format!(" {}.{} ({})", t.schema, t.name, t.kind)); - count += 1; + if !dense || !self.types.is_empty() { + sep(&mut output, !self.types.is_empty()); + output.push(" Types:".to_string()); + if self.types.is_empty() { + output.push(" (none)".to_string()); + } else { + for t in &self.types { + output.push(format!(" {}.{} ({})", t.schema, t.name, t.kind)); + count += 1; + } } } // Summary - output.push(String::new()); + if !dense { + output.push(String::new()); + } output.push(format!( "{} objects that {}.{} depends on", count, schema, table @@ -1182,18 +1238,37 @@ impl Dependencies { // ============================================================================ impl TableDescribe { - /// Format the describe output for display - /// If verbose is true, includes additional details and auto-vacuum/auto-analyze timestamps - pub fn format(&self, verbose: bool) -> String { + /// Format the describe output for display. + /// + /// `verbose` includes details and auto-vacuum/auto-analyze timestamps. + /// `density` selects the layout: `Pretty` pads columns/constraints to width + /// and separates sections with blank lines; `Dense` single-spaces fields, + /// drops the blank-line separators, and omits empty `(none)` sections — the + /// section labels stay identical so the output is still self-describing. + pub fn format(&self, verbose: bool, density: crate::output::Density) -> String { + let dense = density.is_dense(); let mut output = Vec::new(); + // Blank line between sections only in pretty mode. + let sep = |out: &mut Vec| { + if !dense { + out.push(String::new()); + } + }; + // Details section (verbose only, at the top) if let Some(ref details) = self.details { output.push("Details:".to_string()); - output.push(format!(" Owner: {}", details.owner)); - output.push(format!(" Type: {}", details.table_kind)); - output.push(format!(" Persistence: {}", details.persistence)); - output.push(String::new()); + if dense { + output.push(format!(" Owner: {}", details.owner)); + output.push(format!(" Type: {}", details.table_kind)); + output.push(format!(" Persistence: {}", details.persistence)); + } else { + output.push(format!(" Owner: {}", details.owner)); + output.push(format!(" Type: {}", details.table_kind)); + output.push(format!(" Persistence: {}", details.persistence)); + } + sep(&mut output); } // Columns section @@ -1201,14 +1276,20 @@ impl TableDescribe { if self.columns.is_empty() { output.push(" (none)".to_string()); } else { - // Calculate column widths for alignment - let max_name = self.columns.iter().map(|c| c.name.len()).max().unwrap_or(0); - let max_type = self - .columns - .iter() - .map(|c| c.data_type.len()) - .max() - .unwrap_or(0); + // Pretty pads name/type to the widest value; dense uses a single + // space so cells aren't surrounded by alignment whitespace. + let (max_name, max_type) = if dense { + (0, 0) + } else { + let n = self.columns.iter().map(|c| c.name.len()).max().unwrap_or(0); + let t = self + .columns + .iter() + .map(|c| c.data_type.len()) + .max() + .unwrap_or(0); + (n, t) + }; for col in &self.columns { let mut parts = Vec::new(); @@ -1248,106 +1329,142 @@ impl TableDescribe { parts.push(format!("DEFAULT {}", default)); } - let suffix = if parts.is_empty() { - String::new() + if dense { + // " name type qualifiers…" — single spaces throughout. + let mut line = format!(" {} {}", col.name, col.data_type); + if !parts.is_empty() { + line.push(' '); + line.push_str(&parts.join(" ")); + } + output.push(line); } else { - format!(" {}", parts.join(" ")) - }; - - output.push(format!( - " {:name_width$} {:type_width$}{}", - col.name, - col.data_type, - suffix, - name_width = max_name, - type_width = max_type - )); + let suffix = if parts.is_empty() { + String::new() + } else { + format!(" {}", parts.join(" ")) + }; + output.push(format!( + " {:name_width$} {:type_width$}{}", + col.name, + col.data_type, + suffix, + name_width = max_name, + type_width = max_type + )); + } } } - // Indexes section - output.push(String::new()); - output.push("Indexes:".to_string()); - if self.indexes.is_empty() { - output.push(" (none)".to_string()); - } else { - // Display canonical definitions from pg_get_indexdef() directly. - // This avoids misleading parsed summaries for complex indexes - // (expression, partial, INCLUDE, etc.) - for idx in &self.indexes { - output.push(format!(" {}", idx.definition)); + // Indexes section. Dense omits the section entirely when empty. + if !dense || !self.indexes.is_empty() { + sep(&mut output); + output.push("Indexes:".to_string()); + if self.indexes.is_empty() { + output.push(" (none)".to_string()); + } else { + // Display canonical definitions from pg_get_indexdef() directly. + // This avoids misleading parsed summaries for complex indexes + // (expression, partial, INCLUDE, etc.) + for idx in &self.indexes { + output.push(format!(" {}", idx.definition)); + } } } // Constraints section - output.push(String::new()); - output.push("Constraints:".to_string()); - if self.constraints.is_empty() { - output.push(" (none)".to_string()); - } else { - let max_name = self - .constraints - .iter() - .map(|c| c.name.len()) - .max() - .unwrap_or(0); - for con in &self.constraints { - output.push(format!( - " {:width$} {}", - con.name, - con.definition, - width = max_name - )); + if !dense || !self.constraints.is_empty() { + sep(&mut output); + output.push("Constraints:".to_string()); + if self.constraints.is_empty() { + output.push(" (none)".to_string()); + } else { + let max_name = if dense { + 0 + } else { + self.constraints + .iter() + .map(|c| c.name.len()) + .max() + .unwrap_or(0) + }; + for con in &self.constraints { + if dense { + output.push(format!(" {} {}", con.name, con.definition)); + } else { + output.push(format!( + " {:width$} {}", + con.name, + con.definition, + width = max_name + )); + } + } } } // Triggers section - output.push(String::new()); - output.push("Triggers:".to_string()); - if self.triggers.is_empty() { - output.push(" (none)".to_string()); - } else { - // Display canonical definitions from pg_get_triggerdef() directly. - // This avoids misleading parsed summaries and ensures correctness - // for all trigger patterns. - for trg in &self.triggers { - output.push(format!(" {}", trg.definition)); + if !dense || !self.triggers.is_empty() { + sep(&mut output); + output.push("Triggers:".to_string()); + if self.triggers.is_empty() { + output.push(" (none)".to_string()); + } else { + // Display canonical definitions from pg_get_triggerdef() directly. + // This avoids misleading parsed summaries and ensures correctness + // for all trigger patterns. + for trg in &self.triggers { + output.push(format!(" {}", trg.definition)); + } } } // Stats section if let Some(ref stats) = self.stats { - output.push(String::new()); + sep(&mut output); output.push("Stats:".to_string()); // Check if stats retrieval failed (e.g., permission denied) if let Some(ref reason) = stats.unavailable_reason { output.push(format!(" {}", reason)); } else { - output.push(format!(" Rows (estimate): ~{}", stats.row_estimate)); - output.push(format!(" Table size: {}", stats.table_size)); - output.push(format!(" Index size: {}", stats.index_size)); - output.push(format!(" Total size: {}", stats.total_size)); + // Labels differ only in trailing alignment padding: pretty pads + // them into a column, dense uses a single space. Every line + // (sizes, timestamps, partition caveat) is present in both forms + // — density never drops information. + let label = |name: &str| { + if dense { + format!(" {}", name) + } else { + format!(" {:width$}", name, width = 18) + } + }; + output.push(format!( + "{}~{}", + label("Rows (estimate):"), + stats.row_estimate + )); + output.push(format!("{}{}", label("Table size:"), stats.table_size)); + output.push(format!("{}{}", label("Index size:"), stats.index_size)); + output.push(format!("{}{}", label("Total size:"), stats.total_size)); if let Some(ref ts) = stats.last_vacuum { - output.push(format!(" Last vacuum: {}", ts)); + output.push(format!("{}{}", label("Last vacuum:"), ts)); } if verbose { if let Some(ref ts) = stats.last_autovacuum { - output.push(format!(" Last autovacuum: {}", ts)); + output.push(format!("{}{}", label("Last autovacuum:"), ts)); } } - if let Some(ref ts) = stats.last_analyze { - output.push(format!(" Last analyze: {}", ts)); + output.push(format!("{}{}", label("Last analyze:"), ts)); } if verbose { if let Some(ref ts) = stats.last_autoanalyze { - output.push(format!(" Last autoanalyze: {}", ts)); + output.push(format!("{}{}", label("Last autoanalyze:"), ts)); } } - // Add caveat for partitioned tables + // Add caveat for partitioned tables (both forms) if stats.is_partitioned { output.push( " (sizes are for parent table only; partitions not included)".to_string(), @@ -1359,7 +1476,7 @@ impl TableDescribe { // RLS section (only if enabled) if let Some(ref rls) = self.rls { if rls.enabled { - output.push(String::new()); + sep(&mut output); let status = if rls.forced { "Row-Level Security: ENABLED (forced)" } else { @@ -1543,7 +1660,7 @@ mod tests { views: vec![], triggers: vec![], }; - let output = deps.format("public", "users"); + let output = deps.format("public", "users", crate::output::Density::Pretty); assert!(output.contains("(none)")); assert!(output.contains("0 objects depend on public.users")); } @@ -1563,7 +1680,7 @@ mod tests { views: vec![], triggers: vec![], }; - let output = deps.format("public", "users"); + let output = deps.format("public", "users", crate::output::Density::Pretty); assert!(output.contains("public.orders.user_id")); assert!(output.contains("→")); assert!(output.contains("1 objects depend on public.users")); @@ -1584,7 +1701,7 @@ mod tests { views: vec![], triggers: vec![], }; - let output = deps.format("app", "parent"); + let output = deps.format("app", "parent", crate::output::Density::Pretty); // Composite FK should show grouped columns assert!(output.contains("(a, b)")); assert!(output.contains("(x, y)")); @@ -1597,7 +1714,7 @@ mod tests { trigger_functions: vec![], types: vec![], }; - let output = deps.format("public", "users"); + let output = deps.format("public", "users", crate::output::Density::Pretty); assert!(output.contains("(none)")); assert!(output.contains("0 objects that public.users depends on")); } @@ -1613,7 +1730,7 @@ mod tests { kind: "enum".to_string(), }], }; - let output = deps.format("public", "tasks"); + let output = deps.format("public", "tasks", crate::output::Density::Pretty); assert!(output.contains("public.status (enum)")); assert!(output.contains("1 objects that public.tasks depends on")); } @@ -1629,7 +1746,7 @@ mod tests { }], types: vec![], }; - let output = deps.format("public", "users"); + let output = deps.format("public", "users", crate::output::Density::Pretty); assert!(output.contains("public.set_updated_at()")); assert!(output.contains("via update_timestamp trigger")); } @@ -1673,7 +1790,7 @@ mod tests { rls: None, }; - let output = table.format(false); + let output = table.format(false, crate::output::Density::Pretty); assert!( output.contains("Constraints:"), "Should have Constraints section" @@ -1714,7 +1831,7 @@ mod tests { rls: None, }; - let output = table.format(false); + let output = table.format(false, crate::output::Density::Pretty); assert!( output.contains("Constraints:"), "Should have Constraints section" @@ -1779,7 +1896,7 @@ mod tests { rls: None, }; - let output = table.format(false); + let output = table.format(false, crate::output::Density::Pretty); // Both constraints should be displayed distinctly assert!( output.contains("PRIMARY KEY (id)"), @@ -1798,4 +1915,104 @@ mod tests { "Should show UNIQUE constraint name" ); } + + /// A small table whose only populated section is Columns. Pretty shows the + /// empty Indexes/Constraints/Triggers sections; dense omits them. + fn columns_only_table() -> TableDescribe { + TableDescribe { + schema: "public".to_string(), + name: "t".to_string(), + columns: vec![ColumnInfo { + name: "id".to_string(), + data_type: "integer".to_string(), + nullable: false, + is_primary_key: true, + identity: None, + is_serial: true, + default: None, + fk_reference: None, + }], + indexes: vec![], + constraints: vec![], + triggers: vec![], + stats: None, + details: None, + rls: None, + } + } + + #[test] + fn dense_drops_decoration_keeps_data() { + use crate::output::Density; + let table = columns_only_table(); + let dense = table.format(false, Density::Dense); + + // Section label and the column data survive. + assert!(dense.contains("Columns:"), "dense keeps section labels"); + assert!( + dense.contains("id integer NOT NULL PRIMARY KEY SERIAL"), + "dense column line is single-spaced, no padding: {dense}" + ); + // Empty sections are omitted entirely in dense mode. + assert!( + !dense.contains("(none)"), + "dense omits empty sections, not '(none)': {dense}" + ); + assert!(!dense.contains("Indexes:"), "dense omits empty Indexes"); + // No blank-line section separators. + assert!( + !dense.contains("\n\n"), + "dense has no blank-line separators: {dense:?}" + ); + } + + #[test] + fn pretty_keeps_empty_sections_and_padding() { + use crate::output::Density; + let table = columns_only_table(); + let pretty = table.format(false, Density::Pretty); + + // Pretty retains the labelled empty sections with their `(none)` marker. + assert!(pretty.contains("Indexes:"), "pretty keeps Indexes section"); + assert!(pretty.contains("(none)"), "pretty marks empty sections"); + assert!( + pretty.contains("\n\n"), + "pretty separates sections with blank lines" + ); + } + + #[test] + fn dense_dependents_omit_empty_subsections() { + use crate::output::Density; + let deps = Dependents { + foreign_keys: vec![ForeignKeyRef { + constraint_name: "orders_user_id_fkey".to_string(), + from_schema: "public".to_string(), + from_table: "orders".to_string(), + from_columns: vec!["user_id".to_string()], + to_schema: "public".to_string(), + to_table: "users".to_string(), + to_columns: vec!["id".to_string()], + }], + views: vec![], + triggers: vec![], + }; + + let dense = deps.format("public", "users", Density::Dense); + assert!( + dense.contains("Foreign Keys"), + "FK subsection kept (present)" + ); + assert!(dense.contains("public.orders.user_id"), "FK row present"); + assert!( + !dense.contains("Views:") && !dense.contains("Triggers:"), + "dense omits empty Views/Triggers subsections: {dense}" + ); + // The summary line is always present. + assert!(dense.contains("1 objects depend on public.users")); + + // Pretty keeps the empty subsections. + let pretty = deps.format("public", "users", Density::Pretty); + assert!(pretty.contains("Views:") && pretty.contains("Triggers:")); + } } diff --git a/src/main.rs b/src/main.rs index de909f5..438abaf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,7 @@ mod suggest; mod tips; use config::Config; use diagnostic::{setup_ctrlc_handler, DiagnosticSession, TimeoutConfig}; -use output::{HelpResponse, JsonError, LlmHelpResponse, Output, VersionResponse}; +use output::{Density, HelpResponse, JsonError, LlmHelpResponse, Output, VersionResponse}; /// Embedded LLM help content (compiled into binary) const LLM_HELP: &str = include_str!("../llms.txt"); @@ -139,6 +139,14 @@ struct Cli { #[arg(long, global = true)] json: bool, + /// Force decorated human output (tables, rules) even when piped + #[arg(long, global = true, conflicts_with = "dense")] + pretty: bool, + + /// Force dense, token-efficient human output even on a terminal + #[arg(long, global = true, conflicts_with = "pretty")] + dense: bool, + /// Path to anonymize rules file (default: ./pgcrate.anonymize.toml) #[arg(long, global = true)] anonymize_config: Option, @@ -1001,7 +1009,13 @@ async fn main() { } }; - let output = Output::new(cli.json, cli.quiet, cli.verbose); + // Readable-output density: TTY → Pretty, piped/captured → Dense, with + // --pretty/--dense overriding detection. Irrelevant in JSON mode. + let density = { + use std::io::IsTerminal; + Density::resolve(cli.pretty, cli.dense, std::io::stdout().is_terminal()) + }; + let output = Output::new(cli.json, density, cli.quiet, cli.verbose); // Gate unsupported commands in JSON mode if cli.json && !json_supported(&cli.command) { @@ -1410,7 +1424,7 @@ async fn run(cli: Cli, output: &Output) -> Result<()> { if cli.json { commands::triage::print_json(&results, timeouts)?; } else { - commands::triage::print_human(&results, cli.quiet); + commands::triage::print_human(&results, cli.quiet, output.density()); } let exit_code = results.exit_code(); @@ -2175,7 +2189,7 @@ async fn run(cli: Cli, output: &Output) -> Result<()> { if cli.json { commands::context::print_json(&result, Some(session.effective_timeouts()))?; } else { - commands::context::print_human(&result); + commands::context::print_human(&result, output.density()); } } Commands::Capabilities => { @@ -2212,7 +2226,7 @@ async fn run(cli: Cli, output: &Output) -> Result<()> { if cli.json { commands::capabilities::print_json(&result, Some(session.effective_timeouts()))?; } else { - commands::capabilities::print_human(&result); + commands::capabilities::print_human(&result, output.density()); } } Commands::Sql { @@ -2246,6 +2260,7 @@ async fn run(cli: Cli, output: &Output) -> Result<()> { timeouts: parse_timeout_config(&cli)?, quiet: cli.quiet, json: cli.json, + density: output.density(), }; commands::sql(&conn_result.url, command.as_deref(), opts).await?; } diff --git a/src/output.rs b/src/output.rs index 12fa83d..3c45a31 100644 --- a/src/output.rs +++ b/src/output.rs @@ -14,27 +14,73 @@ pub enum OutputMode { Json, } +/// Readable-output density for human (non-JSON) mode. +/// +/// `Pretty` is the decorated form a person reads at a terminal — box rules, +/// columns padded to width, emoji status glyphs. `Dense` carries the *same +/// information* with the decoration stripped: no rules, no padding, no glyphs. +/// It exists because agents pay per token for the output they capture, and a +/// padded table is mostly whitespace. +/// +/// Mode is chosen once in `main` from `stdout.is_terminal()` (TTY → Pretty, +/// piped → Dense) with `--pretty`/`--dense` overriding detection. JSON mode +/// ignores density entirely. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Density { + Pretty, + Dense, +} + +impl Density { + /// Resolve density from TTY detection plus mutually-exclusive overrides. + /// `pretty`/`dense` come from the global CLI flags; clap guarantees they + /// are not both set. With neither, a terminal gets `Pretty` and anything + /// piped/captured gets `Dense`. + pub fn resolve(pretty: bool, dense: bool, stdout_is_terminal: bool) -> Self { + if pretty { + Density::Pretty + } else if dense { + Density::Dense + } else if stdout_is_terminal { + Density::Pretty + } else { + Density::Dense + } + } + + pub fn is_dense(self) -> bool { + self == Density::Dense + } +} + /// Output helper that centralizes all CLI output #[derive(Debug, Clone)] pub struct Output { pub mode: OutputMode, + pub density: Density, pub quiet: bool, pub verbose: bool, } impl Output { - pub fn new(json: bool, quiet: bool, verbose: bool) -> Self { + pub fn new(json: bool, density: Density, quiet: bool, verbose: bool) -> Self { Self { mode: if json { OutputMode::Json } else { OutputMode::Human }, + density, quiet, verbose, } } + /// Readable-output density (Pretty vs Dense). Meaningless in JSON mode. + pub fn density(&self) -> Density { + self.density + } + /// Write data to stdout (the command's "answer") /// In JSON mode, this is the only thing that goes to stdout #[allow(dead_code)] @@ -531,21 +577,36 @@ mod tests { #[test] fn test_output_mode_json() { - let output = Output::new(true, false, false); + let output = Output::new(true, Density::Dense, false, false); assert!(output.is_json()); assert_eq!(output.mode, OutputMode::Json); } #[test] fn test_output_mode_human() { - let output = Output::new(false, false, false); + let output = Output::new(false, Density::Pretty, false, false); assert!(!output.is_json()); assert_eq!(output.mode, OutputMode::Human); } #[test] fn test_output_quiet() { - let output = Output::new(false, true, false); + let output = Output::new(false, Density::Dense, true, false); assert!(output.is_quiet()); } + + #[test] + fn test_density_resolve_detection() { + // No overrides: TTY → Pretty, piped → Dense. + assert_eq!(Density::resolve(false, false, true), Density::Pretty); + assert_eq!(Density::resolve(false, false, false), Density::Dense); + } + + #[test] + fn test_density_resolve_overrides_beat_detection() { + // --pretty forces Pretty even when piped. + assert_eq!(Density::resolve(true, false, false), Density::Pretty); + // --dense forces Dense even on a terminal. + assert_eq!(Density::resolve(false, true, true), Density::Dense); + } } diff --git a/tests/density_integration.rs b/tests/density_integration.rs new file mode 100644 index 0000000..f6ab6bd --- /dev/null +++ b/tests/density_integration.rs @@ -0,0 +1,257 @@ +//! Integration tests for output density (PGC-102). +//! +//! Verifies the TTY-aware readable-output behavior end to end: a piped +//! invocation (which every test is, since stdout is a pipe) defaults to the +//! dense format, `--pretty` restores the decorated form, `--dense` is honored +//! on top of detection, and `--json` is untouched by any of it. The core +//! guarantee under test is *zero information loss* between the two forms. +//! +//! These tests require a running PostgreSQL instance. +//! Set TEST_DATABASE_URL or use the default postgres://localhost/postgres. +//! +//! Run with: cargo test --test density_integration + +use std::env; +use std::process::Command; +use std::sync::atomic::{AtomicU32, Ordering}; + +static TEST_COUNTER: AtomicU32 = AtomicU32::new(0); + +fn get_test_db_url() -> String { + env::var("TEST_DATABASE_URL").unwrap_or_else(|_| "postgres://localhost/postgres".to_string()) +} + +fn pgcrate_binary() -> String { + env!("CARGO_BIN_EXE_pgcrate").to_string() +} + +/// Run pgcrate via the compiled binary. stdout is a pipe here, so without an +/// override the binary picks the dense format — exactly the agent-capture case. +fn run_pgcrate(args: &[&str], db_url: &str) -> std::process::Output { + Command::new(pgcrate_binary()) + .args(args) + .env("DATABASE_URL", db_url) + .output() + .expect("Failed to execute pgcrate") +} + +fn run_psql(sql: &str, db_url: &str) -> std::process::Output { + Command::new("psql") + .args([db_url, "-c", sql]) + .output() + .expect("Failed to execute psql") +} + +fn unique_db_name(base: &str) -> String { + let id = TEST_COUNTER.fetch_add(1, Ordering::SeqCst); + let pid = std::process::id(); + format!("{}_{pid}_{id}", base) +} + +fn setup_test_db(base_name: &str) -> Option { + let test_db = unique_db_name(base_name); + let db_url = get_test_db_url(); + let test_url = db_url + .rsplit_once('/') + .map(|(base, _)| format!("{}/{}", base, test_db)) + .unwrap_or_else(|| format!("{}/{}", db_url, test_db)); + + let _ = run_psql(&format!("DROP DATABASE IF EXISTS \"{}\"", test_db), &db_url); + let create_result = run_psql(&format!("CREATE DATABASE \"{}\"", test_db), &db_url); + if !create_result.status.success() { + eprintln!("Skipping test: could not create test database"); + return None; + } + Some(test_url) +} + +fn cleanup_test_db(test_url: &str) { + let db_url = get_test_db_url(); + if let Some(db_name) = test_url.rsplit('/').next() { + let _ = run_psql(&format!("DROP DATABASE IF EXISTS \"{}\"", db_name), &db_url); + } +} + +fn out(o: &std::process::Output) -> String { + String::from_utf8_lossy(&o.stdout).to_string() +} + +// =========================================================================== +// context +// =========================================================================== + +#[test] +fn context_piped_defaults_to_dense() { + let db_url = get_test_db_url(); + let stdout = out(&run_pgcrate(&["context"], &db_url)); + + // Dense folds the connection onto a single line: "CONNECTION: user@host…". + assert!( + stdout.contains("CONNECTION: ") && stdout.contains('@'), + "piped context should be dense (single-line CONNECTION): {stdout}" + ); + // The padded, multi-line " Host:" form must NOT appear when dense. + assert!( + !stdout.contains(" Host:"), + "dense context drops the padded Host: line: {stdout}" + ); + // Information is still present. + assert!(stdout.contains("SERVER:"), "dense keeps SERVER section"); + assert!(stdout.contains("PRIVILEGES:"), "dense keeps PRIVILEGES"); +} + +#[test] +fn context_pretty_override_restores_decorated_form() { + let db_url = get_test_db_url(); + let stdout = out(&run_pgcrate(&["context", "--pretty"], &db_url)); + + // --pretty forces the multi-line padded layout even though stdout is piped. + assert!( + stdout.contains("CONNECTION:\n") || stdout.contains(" Host:"), + "--pretty restores the multi-line form: {stdout}" + ); + assert!(stdout.contains(" Host:"), "pretty shows the Host: field"); + assert!(stdout.contains(" Port:"), "pretty shows the Port: field"); +} + +#[test] +fn context_dense_and_pretty_carry_same_information() { + let db_url = get_test_db_url(); + let dense = out(&run_pgcrate(&["context", "--dense"], &db_url)); + let pretty = out(&run_pgcrate(&["context", "--pretty"], &db_url)); + + // Both forms must report every section — density changes layout, not facts. + for section in ["CONNECTION", "SERVER", "EXTENSIONS", "PRIVILEGES"] { + assert!(dense.contains(section), "dense missing {section}: {dense}"); + assert!( + pretty.contains(section), + "pretty missing {section}: {pretty}" + ); + } + // Both must name the superuser privilege state (yes/no in dense, ✓/✗ pretty). + assert!( + dense.contains("superuser="), + "dense reports superuser state" + ); + assert!( + pretty.contains("Superuser:"), + "pretty reports superuser state" + ); +} + +#[test] +fn context_json_is_unaffected_by_density_flags() { + let db_url = get_test_db_url(); + // --json wins over --dense/--pretty and emits the structured envelope. + let stdout = out(&run_pgcrate(&["context", "--json", "--dense"], &db_url)); + let json: serde_json::Value = serde_json::from_str(&stdout) + .expect("context --json must be valid JSON regardless of density"); + assert_eq!(json.get("ok"), Some(&serde_json::json!(true))); + assert_eq!( + json.get("schema_id"), + Some(&serde_json::json!("pgcrate.diagnostics.context")) + ); +} + +// =========================================================================== +// inspect table +// =========================================================================== + +#[test] +fn inspect_table_dense_drops_decoration_keeps_data() { + let Some(test_url) = setup_test_db("pgcrate_density_inspect") else { + return; + }; + let setup = r#" + CREATE TABLE accounts ( + id SERIAL PRIMARY KEY, + email TEXT NOT NULL UNIQUE, + balance NUMERIC(12,2) NOT NULL DEFAULT 0 + ); + CREATE INDEX idx_accounts_email ON accounts(email); + "#; + assert!(run_psql(setup, &test_url).status.success(), "setup failed"); + + let dense = out(&run_pgcrate( + &["inspect", "table", "public.accounts"], + &test_url, + )); + + // Section labels and data survive the dense form. + assert!(dense.contains("Table: \"public\".\"accounts\"")); + assert!(dense.contains("Columns:"), "dense keeps Columns label"); + assert!( + dense.contains("email text NOT NULL"), + "dense single-spaces cells" + ); + assert!( + dense.contains("Constraints:"), + "dense keeps Constraints label" + ); + + // Decoration the agent doesn't need is gone: the ── rule and empty sections. + assert!(!dense.contains('─'), "dense drops the box rule: {dense}"); + assert!( + !dense.contains("Triggers:"), + "dense omits the empty Triggers section: {dense}" + ); + + // The same table under --pretty keeps the rule and the empty section. + let pretty = out(&run_pgcrate( + &["inspect", "table", "public.accounts", "--pretty"], + &test_url, + )); + assert!(pretty.contains('─'), "pretty keeps the box rule"); + assert!( + pretty.contains("Triggers:"), + "pretty keeps the empty Triggers section" + ); + + cleanup_test_db(&test_url); +} + +// =========================================================================== +// dba triage +// =========================================================================== + +#[test] +fn triage_dense_drops_padding_and_emoji() { + let db_url = get_test_db_url(); + let stdout = out(&run_pgcrate(&["dba", "triage"], &db_url)); + + // Dense triage is one "LABEL: summary — status" line per check, no emoji. + assert!( + stdout.contains("BLOCKING LOCKS:"), + "dense triage labels each check: {stdout}" + ); + assert!( + stdout.contains("healthy") || stdout.contains("WARNING") || stdout.contains("CRITICAL"), + "dense triage carries the status word: {stdout}" + ); + assert!( + !stdout.contains('✓'), + "dense triage drops the status emoji: {stdout}" + ); +} + +// =========================================================================== +// capabilities +// =========================================================================== + +#[test] +fn capabilities_dense_drops_glyphs_keeps_ids() { + let db_url = get_test_db_url(); + let stdout = out(&run_pgcrate(&["capabilities"], &db_url)); + + assert!(stdout.contains("CAPABILITIES:"), "header present"); + assert!( + stdout.contains("diagnostics.triage available"), + "dense lists capability id + status word: {stdout}" + ); + assert!(stdout.contains("SUMMARY:"), "summary line present"); + // No glyph markers in dense mode. + assert!( + !stdout.contains('✓') && !stdout.contains('✗') && !stdout.contains('⚠'), + "dense capabilities drop status glyphs: {stdout}" + ); +}