diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5b9663401..1f978b56b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `read` against a SQLite table with many columns (e.g. 33) rendering every cell as an ellipsis and chopping the right edge. The ASCII table shrinker bottomed out at `MIN_COLUMN_WIDTH=1` so every multi-char cell collapsed to `…`, and the final line truncation then cut off the right side. The renderer now bumps the per-column floor to 3 and falls back to a vertical `column: value` block layout per row when the column count exceeds the horizontal width budget. ([#3107](https://github.com/can1357/oh-my-pi/issues/3107)) + ## [16.1.7] - 2026-06-20 ### Fixed diff --git a/packages/coding-agent/src/tools/sqlite-reader.ts b/packages/coding-agent/src/tools/sqlite-reader.ts index dbb637712..a7ced973e 100644 --- a/packages/coding-agent/src/tools/sqlite-reader.ts +++ b/packages/coding-agent/src/tools/sqlite-reader.ts @@ -21,7 +21,19 @@ const MAX_QUERY_LIMIT = 500; export const MAX_RAW_QUERY_ROWS = 1000; const MAX_RENDER_WIDTH = 120; const MAX_COLUMN_WIDTH = 40; -const MIN_COLUMN_WIDTH = 1; +/** + * Floor for each ASCII-table column. At width 2 (or 1) every multi-char cell + * collapses to a lone ellipsis, so the renderer keeps each column wide enough + * to show at least one real glyph alongside the ellipsis (e.g. `Fo…`). When a + * row has too many columns to honor this floor inside `MAX_RENDER_WIDTH`, + * `buildAsciiTable` falls back to per-row vertical blocks via + * {@link buildVerticalBlocks} — issue #3107. + */ +const MIN_COLUMN_WIDTH = 3; +/** Separator overhead per column in the ASCII table (`" | "`). */ +const COLUMN_SEPARATOR_WIDTH = 3; +/** Constant frame overhead added once to every row (leading `"|"` + trailing `" |"` after the per-column accounting). */ +const TABLE_FRAME_WIDTH = 1; /** * Upper bound on rows scanned when counting a table for the listing. SQLite has * no stored row count, so `COUNT(*)` is a full b-tree scan — multi-second on a @@ -142,10 +154,53 @@ function padCell(value: string, width: number): string { return `${truncated}${" ".repeat(width - visibleWidth)}`; } +/** + * Width budget the ASCII layout needs at the floor (each column at + * `MIN_COLUMN_WIDTH`). When this exceeds `MAX_RENDER_WIDTH`, no choice of + * per-column widths can fit the header inside the budget — every cell is then + * forced down to width 1 by the shrink loop, rendering as a lone ellipsis, and + * the right edge is still chopped by the final per-line truncation (#3107). + */ +function tableFitsAtMinimum(columnCount: number): boolean { + return MIN_COLUMN_WIDTH * columnCount + COLUMN_SEPARATOR_WIDTH * columnCount + TABLE_FRAME_WIDTH <= MAX_RENDER_WIDTH; +} + +/** + * Vertical fallback used when a table has too many columns to fit horizontally + * (>19 at the default 120-cell budget). Each row becomes a labelled block of + * `column: value` lines, mirroring `psql`'s expanded display mode. Column + * names are right-padded so colons align; the value is left raw and the whole + * line is truncated at `MAX_RENDER_WIDTH`. + */ +function buildVerticalBlocks(columns: string[], rows: SqliteRow[]): string { + if (rows.length === 0) { + return "(no rows)"; + } + let nameWidth = MIN_COLUMN_WIDTH; + for (const column of columns) { + nameWidth = Math.max(nameWidth, Bun.stringWidth(sanitizeCell(column))); + } + nameWidth = Math.min(MAX_COLUMN_WIDTH, nameWidth); + return rows + .map((row, index) => { + const block = [`── Row ${index + 1} ──`]; + for (const column of columns) { + const name = padCell(column, nameWidth); + const value = sanitizeCell(stringifySqliteValue(row[column])); + block.push(truncateToWidth(`${name}: ${value}`, MAX_RENDER_WIDTH)); + } + return block.join("\n"); + }) + .join("\n\n"); +} + function buildAsciiTable(columns: string[], rows: SqliteRow[]): string { if (columns.length === 0) { return rows.length === 0 ? "(no rows)" : "(rows returned without named columns)"; } + if (!tableFitsAtMinimum(columns.length)) { + return buildVerticalBlocks(columns, rows); + } const widths = columns.map(column => Math.max(MIN_COLUMN_WIDTH, Math.min(MAX_COLUMN_WIDTH, Bun.stringWidth(sanitizeCell(column)))), @@ -157,7 +212,8 @@ function buildAsciiTable(columns: string[], rows: SqliteRow[]): string { } } - let totalWidth = widths.reduce((sum, width) => sum + width, 0) + columns.length * 3 + 1; + const overhead = columns.length * COLUMN_SEPARATOR_WIDTH + TABLE_FRAME_WIDTH; + let totalWidth = widths.reduce((sum, width) => sum + width, 0) + overhead; while (totalWidth > MAX_RENDER_WIDTH) { let widestIndex = -1; let widestWidth = MIN_COLUMN_WIDTH; @@ -169,7 +225,7 @@ function buildAsciiTable(columns: string[], rows: SqliteRow[]): string { } if (widestIndex === -1) break; widths[widestIndex] = Math.max(MIN_COLUMN_WIDTH, (widths[widestIndex] ?? MIN_COLUMN_WIDTH) - 1); - totalWidth = widths.reduce((sum, width) => sum + width, 0) + columns.length * 3 + 1; + totalWidth = widths.reduce((sum, width) => sum + width, 0) + overhead; } const header = `| ${columns.map((column, index) => padCell(column, widths[index] ?? MIN_COLUMN_WIDTH)).join(" | ")} |`; diff --git a/packages/coding-agent/test/tools/sqlite.test.ts b/packages/coding-agent/test/tools/sqlite.test.ts index a2652bf9e..eda7a0dd2 100644 --- a/packages/coding-agent/test/tools/sqlite.test.ts +++ b/packages/coding-agent/test/tools/sqlite.test.ts @@ -370,6 +370,36 @@ describe("SQLite tool support", () => { } }); + it("falls back to vertical row blocks when the column count exceeds the horizontal budget (#3107)", () => { + const columns = ["_id", ...Array.from({ length: 32 }, (_, i) => `col_${i + 1}`)]; + const row: Record = { _id: 7 }; + for (let i = 1; i <= 32; i++) row[`col_${i}`] = `value_${i}`; + + const rendered = renderTable(columns, [row], { + totalCount: 1, + offset: 0, + limit: 20, + table: "wide_columns", + dbPath: sqlitePath, + }); + + // Horizontal layout would shrink every column to width 1 and chop the + // right edge — i.e., the line would look like `| … | … | … | …` (>=2 + // ellipses chained by ` | `). Vertical mode renders one `col: value` + // per line, so that signature must NOT be present. + expect(rendered).not.toMatch(/…(?: \| …){2,}/); + + // Each declared column must appear with its real value on its own line. + expect(rendered).toContain("── Row 1 ──"); + expect(rendered).toContain("_id : 7"); + expect(rendered).toContain("col_1 : value_1"); + expect(rendered).toContain("col_32: value_32"); + + for (const line of rendered.split("\n")) { + expect(Bun.stringWidth(line)).toBeLessThanOrEqual(120); + } + }); + it("inserts rows through the write tool with JSON5 content", async () => { const dbPath = await stampFreshDb("write-insert.sqlite"); await writeTool.execute("sqlite-write-insert", {