Skip to content

Conversation

@mk3008
Copy link
Owner

@mk3008 mk3008 commented Oct 26, 2025

Introduce InsertClause for better handling of INSERT statements, enhance SQL formatting options with insertColumnsOneLine, and add tests for various formatting scenarios.

Summary by CodeRabbit

  • New Features

    • Added insertColumnsOneLine option to keep INSERT column lists on a single line.
  • Documentation

    • Added "INSERT column list layouts" guide with examples and interaction notes for comma-related options.
    • Expanded demo descriptive copy to reference DML/DDL beyond SELECT.
  • Improvements

    • Demo: multi-statement parsing/analysis, per-statement formatting, better error and empty-state messages.
    • Tests: added insert-formatting tests and small test cleanups.

- Introduced `InsertClause` to `SqlPrintTokenContainerType` for better handling of INSERT statements.
- Updated `SqlPrintTokenParser` to create tokens for the INSERT clause.
- Enhanced `SqlFormatter` with `insertColumnsOneLine` option to control single-line formatting of column lists.
- Modified `SqlPrinter` to respect the new formatting option and improve handling of commas and spaces in INSERT statements.
- Added tests for various INSERT formatting scenarios, including single-line column lists and preservation of comments in VALUES rows.
@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

Walkthrough

Adds a new public option insertColumnsOneLine and oneline formatting helper to control INSERT column-list layout, updates token model to include an InsertClause container, integrates oneline logic into the printer, and refactors demo code for multi-statement parsing with caching and improved error handling.

Changes

Cohort / File(s) Summary
Documentation
docs/guide/formatting-recipes.md, docs/public/demo/index.html
Documented the new insertColumnsOneLine option and added "INSERT column list layouts" section; expanded descriptive copy in demo index.
Demo UI & Parsing
docs/public/demo/analysis-features.js, docs/public/demo/script.js, docs/public/demo/*
Replaced remote rawsql-ts imports with local vendor bundle, added parse caching, multi-statement splitting, SELECT-segmentation helpers, and refactored table/CTE/schema extraction with standardized messages and improved error handling.
Token Model
packages/core/src/models/SqlPrintToken.ts
Added InsertClause to SqlPrintTokenContainerType enum.
Token Parser
packages/core/src/parsers/SqlPrintTokenParser.ts
visitInsertClause now creates an InsertClause container token (passes container type when constructing tokens).
Oneline Formatting Helper
packages/core/src/transformers/OnelineFormattingHelper.ts
New exports: CommaBreakStyle, OnelineFormattingOptions, and OnelineFormattingHelper class encapsulating oneline decisions, comma-break resolution, INSERT token formatting, and whitespace handling.
Formatter API
packages/core/src/transformers/SqlFormatter.ts
Added optional insertColumnsOneLine?: boolean to BaseFormattingOptions.
SqlPrinter & Oneline Integration
packages/core/src/transformers/SqlPrinter.ts
Integrated OnelineFormattingHelper; added insertColumnsOneLine option and onelineHelper property; refactored oneline/container/comma/space/parenthesis handling, added helpers (e.g., isInsertClauseOneline, tryAppendInsertClauseTokenText) and updated oneline printer creation paths. Public types expose helper CommaBreakStyle alias and SqlPrinterOptions.insertColumnsOneLine.
Tests — Insert Formatting
packages/core/tests/SqlFormatter.insert.test.ts
New Vitest suite covering multiline column lists, comment preservation with exportComment, and single-line column lists via insertColumnsOneLine.
Tests — Minor Fixes
packages/core/tests/parsers/InsertQueryParser.test.ts, packages/core/tests/transformers/CTEQueryDecomposer.test.ts
Removed scaffolding comments in InsertQueryParser tests; adjusted expected whitespace and added trailing newline in CTEQueryDecomposer test.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as FormatterCaller
    participant SqlFormatter
    participant SqlPrinter
    participant OnelineHelper
    participant Output as FormattedSQL

    Caller->>SqlFormatter: format(ast, options { insertColumnsOneLine })
    SqlFormatter->>SqlPrinter: new SqlPrinter(options)
    SqlPrinter->>OnelineHelper: new OnelineFormattingHelper(options)

    loop token stream
        SqlPrinter->>SqlPrinter: visitToken(token, parentType)
        alt token is InsertClause
            SqlPrinter->>OnelineHelper: isInsertClauseOneline(parentType)
            OnelineHelper-->>SqlPrinter: boolean
            SqlPrinter->>OnelineHelper: formatInsertClauseToken(...)
            OnelineHelper-->>SqlPrinter: formattedText / handled
        end
        alt token involves comma logic
            SqlPrinter->>OnelineHelper: resolveCommaBreak(parentType, commaBreak, cteCommaBreak, valuesCommaBreak)
            OnelineHelper-->>SqlPrinter: resolvedStyle
        end
        alt space/paren decisions
            SqlPrinter->>OnelineHelper: shouldSkipInsertClauseSpace(...) / shouldBreakAfterOpeningParen(...)
            OnelineHelper-->>SqlPrinter: boolean
        end
    end

    SqlPrinter-->>Output: combined formatted SQL
    Output-->>Caller: result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–75 minutes

  • Pay special attention to:
    • OnelineFormattingHelper methods (comma-break resolution, insert token formatting) and edge cases.
    • SqlPrinter changes: token visitation flow, parentContainerType propagation, and interactions between insertColumnsOneLine, commaBreak, and valuesCommaBreak.
    • Demo parsing changes: multi-statement splitting, caching, and error messaging.

Possibly related PRs

Poem

🐇 I nibble tokens, hop and twine,
Columns lined up, one or many in a line,
With helper hops I tidy commas bright,
Whether compact or spread—both look right—
A rabbit's cheer for cleaner SQL tonight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Add support for INSERT clause formatting options" is clearly related to the main changes in this pull request. The PR introduces a new public formatting option called insertColumnsOneLine that controls how INSERT column lists are formatted, adds the InsertClause container type for better INSERT statement handling, and includes comprehensive tests and documentation for these formatting scenarios. The title accurately captures the primary objective stated in the PR summary, which is to enhance SQL formatting options specifically for INSERT statements. While the changeset includes supporting infrastructure like the OnelineFormattingHelper class and demo file refactoring, these are in service of the main goal, and the title appropriately highlights the user-facing feature being added.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch insert_query_format

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/core/tests/transformers/CTEQueryDecomposer.test.ts (1)

296-304: Use SqlFormatter for SQL comparison and remove console logs

Direct string equality here is brittle; normalize both sides via SqlFormatter. Also, console.log calls should be removed per guidelines.

Apply this diff within the test:

-            console.log("=== Generated SQL Full Text ===");
-            console.log(filteredData?.query);
-            console.log("=== End of Generated SQL ===");
-
-            // Expected format: comments should appear before WITH clause
-            const expectedSql = `/* Auto-generated by CTE decomposer */ /* Original CTE: filtered_data */ /* Dependencies: base_data */ /* Dependents: none */ with base_data as (select * from users) select * from base_data where active = true`;
-            
-            expect(filteredData?.query).toBe(expectedSql);
+            // Expected format: comments appear before WITH clause; compare normalized SQL
+            const expectedSql = `/* Auto-generated by CTE decomposer */ /* Original CTE: filtered_data */ /* Dependencies: base_data */ /* Dependents: none */ with base_data as (select * from users) select * from base_data where active = true`;
+            const { SqlFormatter, SqlParser } = require('rawsql-ts');
+            const fmt = new SqlFormatter({ newline: 'space', identifierEscape: { start: '', end: '' }, commentStyle: 'block' });
+            const actualNorm = fmt.format(SqlParser.parse(filteredData!.query)).formattedSql;
+            const expectedNorm = fmt.format(SqlParser.parse(expectedSql)).formattedSql;
+            expect(actualNorm).toBe(expectedNorm);

If ESM-only in tests, replace require with import at top:

import { SqlFormatter, SqlParser } from 'rawsql-ts';

As per coding guidelines.

packages/core/src/transformers/SqlPrinter.ts (1)

273-276: Bug: comparing containerType to string prevents JOIN handling

containerType is an enum; comparing to the string literal "JoinClause" prevents this branch from executing. Replace with the enum value SqlPrintTokenContainerType.JoinClause.

-        } else if (token.containerType === "JoinClause") {
+        } else if (token.containerType === SqlPrintTokenContainerType.JoinClause) {
             this.handleJoinClauseToken(token, level);
🧹 Nitpick comments (7)
docs/public/demo/analysis-features.js (2)

125-129: Deterministic ordering for lists

Sort names before rendering to keep UI stable and copy-friendly.

-        Array.from(tableNames).forEach(tableName => {
+        Array.from(tableNames).sort((a,b)=>a.localeCompare(b)).forEach(tableName => {
           const li = document.createElement('li');
           li.textContent = tableName;
           tableListElement.appendChild(li);
         });
-        Array.from(cteNames).forEach(name => {
+        Array.from(cteNames).sort((a,b)=>a.localeCompare(b)).forEach(name => {
           const listItem = document.createElement('li');
           listItem.textContent = name;
           cteListElement.appendChild(listItem);
         });

Also applies to: 191-195


33-63: Parse cache: surface aggregated errors when nothing parses

If all segments fail to parse, propagate a concise aggregated error so the UI can hint at failure instead of showing “no SELECT statements”.

Example tweak:

  • When statements.size === 0 and errors.length > 0, throw a combined Error with first few messages; catch upstream to render it.
packages/core/src/transformers/SqlFormatter.ts (1)

77-79: Option added correctly; consider explicit pass‑through to SqlPrinter for clarity

insertColumnsOneLine is included via the spread, but mirroring nearby explicit mappings avoids accidental regressions when refactoring.

Apply this diff in the SqlPrinter construction:

         this.printer = new SqlPrinter({
             ...options,
             parenthesesOneLine: options.parenthesesOneLine,
             betweenOneLine: options.betweenOneLine,
             valuesOneLine: options.valuesOneLine,
             joinOneLine: options.joinOneLine,
             caseOneLine: options.caseOneLine,
             subqueryOneLine: options.subqueryOneLine,
+            insertColumnsOneLine: options.insertColumnsOneLine,
             indentNestedParentheses: options.indentNestedParentheses
         });
packages/core/tests/SqlFormatter.insert.test.ts (1)

68-77: Make comment assertions less brittle

Counting two occurrences can flap if spacing changes but semantics stay. Consider asserting presence and relative placement instead of exact count to reduce false negatives, while keeping the same expected strings.

Example:

expect(formattedSql).toMatch(/\(1,\s*10\)\s*\/\* first row \*\//);
expect(formattedSql).toMatch(/,\s*\(2,\s*20\)\s*\/\* second row \*\//);

Based on learnings.

packages/core/src/parsers/SqlPrintTokenParser.ts (1)

2368-2386: Include InsertClause in clause‑level containers for consistent comment spacing

Now that InsertClause is a proper container, treat it as clause‑level so insert‑leading comments get a separator space like SELECT/WHERE.

Apply this diff in isClauseLevelContainer:

     private isClauseLevelContainer(containerType: SqlPrintTokenContainerType): boolean {
         switch (containerType) {
             case SqlPrintTokenContainerType.SelectClause:
             case SqlPrintTokenContainerType.FromClause:
             case SqlPrintTokenContainerType.WhereClause:
             case SqlPrintTokenContainerType.GroupByClause:
             case SqlPrintTokenContainerType.HavingClause:
             case SqlPrintTokenContainerType.OrderByClause:
             case SqlPrintTokenContainerType.LimitClause:
             case SqlPrintTokenContainerType.OffsetClause:
             case SqlPrintTokenContainerType.WithClause:
             case SqlPrintTokenContainerType.SimpleSelectQuery:
+            case SqlPrintTokenContainerType.InsertClause:
                 return true;
             default:
                 return false;
         }
     }
docs/public/demo/script.js (1)

227-227: Avoid noisy logging in demo (optional)

Console logging Mermaid code can be verbose; guard behind a debug flag or remove.

-        console.log('Generated Mermaid code:', mermaidCode);
+        // if (window.DEBUG) console.debug('Generated Mermaid code:', mermaidCode);
packages/core/src/transformers/SqlPrinter.ts (1)

1112-1116: Unused helper can be removed or documented

getClauseBreakIndentLevel isn’t referenced. Remove to reduce noise or add a call site if planned.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 385af84 and 025c85a.

📒 Files selected for processing (12)
  • docs/guide/formatting-recipes.md (2 hunks)
  • docs/public/demo/analysis-features.js (6 hunks)
  • docs/public/demo/index.html (1 hunks)
  • docs/public/demo/script.js (5 hunks)
  • packages/core/src/models/SqlPrintToken.ts (1 hunks)
  • packages/core/src/parsers/SqlPrintTokenParser.ts (1 hunks)
  • packages/core/src/transformers/OnelineFormattingHelper.ts (1 hunks)
  • packages/core/src/transformers/SqlFormatter.ts (1 hunks)
  • packages/core/src/transformers/SqlPrinter.ts (15 hunks)
  • packages/core/tests/SqlFormatter.insert.test.ts (1 hunks)
  • packages/core/tests/parsers/InsertQueryParser.test.ts (0 hunks)
  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • packages/core/tests/parsers/InsertQueryParser.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
packages/core/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

packages/core/**/*.{ts,tsx}: Always compile and fix TypeScript errors before running tests
Source code comments must be written in English only
For model-driven mappings (mapping.typeInfo and mapping.structure), use convertModelDrivenMapping
Use imports from 'rawsql-ts' rather than local relative paths for library usage

Files:

  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts
  • packages/core/src/transformers/SqlFormatter.ts
  • packages/core/src/models/SqlPrintToken.ts
  • packages/core/src/parsers/SqlPrintTokenParser.ts
  • packages/core/src/transformers/OnelineFormattingHelper.ts
  • packages/core/tests/SqlFormatter.insert.test.ts
  • packages/core/src/transformers/SqlPrinter.ts
packages/core/**/*.{test,spec}.{ts,tsx,js}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

Tests are specs — never change expected values without user consultation

Files:

  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts
  • packages/core/tests/SqlFormatter.insert.test.ts
packages/core/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

Remove console.log statements before commit

Files:

  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts
  • packages/core/src/transformers/SqlFormatter.ts
  • packages/core/src/models/SqlPrintToken.ts
  • packages/core/src/parsers/SqlPrintTokenParser.ts
  • packages/core/src/transformers/OnelineFormattingHelper.ts
  • packages/core/tests/SqlFormatter.insert.test.ts
  • packages/core/src/transformers/SqlPrinter.ts
packages/core/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

Use SqlFormatter for SQL comparison tests

Files:

  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts
  • packages/core/tests/SqlFormatter.insert.test.ts
🧠 Learnings (2)
📚 Learning: 2025-10-18T02:29:34.428Z
Learnt from: CR
PR: mk3008/rawsql-ts#0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-10-18T02:29:34.428Z
Learning: Applies to packages/core/**/*.{test,spec}.{ts,tsx} : Use SqlFormatter for SQL comparison tests

Applied to files:

  • packages/core/tests/transformers/CTEQueryDecomposer.test.ts
  • packages/core/tests/SqlFormatter.insert.test.ts
📚 Learning: 2025-10-18T02:29:34.428Z
Learnt from: CR
PR: mk3008/rawsql-ts#0
File: packages/core/AGENTS.md:0-0
Timestamp: 2025-10-18T02:29:34.428Z
Learning: Applies to packages/core/**/*.{ts,tsx} : Use imports from 'rawsql-ts' rather than local relative paths for library usage

Applied to files:

  • docs/public/demo/script.js
🧬 Code graph analysis (6)
packages/core/src/parsers/SqlPrintTokenParser.ts (1)
packages/core/src/models/SqlPrintToken.ts (1)
  • SqlPrintToken (111-141)
packages/core/src/transformers/OnelineFormattingHelper.ts (3)
packages/core/src/transformers/SqlPrinter.ts (2)
  • CommaBreakStyle (17-17)
  • ensureTrailingSpace (426-435)
packages/core/src/transformers/SqlFormatter.ts (1)
  • WithClauseStyle (18-18)
packages/core/src/models/SqlPrintToken.ts (1)
  • SqlPrintToken (111-141)
docs/public/demo/analysis-features.js (5)
docs/public/demo/script.js (10)
  • sqlText (210-210)
  • sqlText (251-251)
  • sqlText (447-447)
  • splitResult (110-110)
  • statements (111-111)
  • errors (112-112)
  • ast (120-120)
  • parsed (167-167)
  • parsed (305-305)
  • tableNames (390-393)
packages/core/src/utils/MultiQuerySplitter.ts (1)
  • MultiQuerySplitter (81-339)
packages/core/src/parsers/SqlParser.ts (1)
  • SqlParser (55-442)
packages/core/src/transformers/TableSourceCollector.ts (1)
  • TableSourceCollector (30-521)
packages/core/src/transformers/CTECollector.ts (1)
  • CTECollector (25-501)
docs/public/demo/script.js (4)
packages/core/src/utils/MultiQuerySplitter.ts (1)
  • MultiQuerySplitter (81-339)
packages/core/src/parsers/SqlParser.ts (1)
  • SqlParser (55-442)
packages/core/src/transformers/QueryFlowDiagramGenerator.ts (1)
  • QueryFlowDiagramGenerator (29-236)
packages/core/src/transformers/SqlFormatter.ts (1)
  • SqlFormatter (116-166)
packages/core/tests/SqlFormatter.insert.test.ts (2)
packages/core/src/parsers/InsertQueryParser.ts (1)
  • InsertQueryParser (14-106)
packages/core/src/transformers/SqlFormatter.ts (1)
  • SqlFormatter (116-166)
packages/core/src/transformers/SqlPrinter.ts (2)
packages/core/src/transformers/OnelineFormattingHelper.ts (3)
  • CommaBreakStyle (4-4)
  • OnelineFormattingHelper (22-130)
  • OnelineFormattingOptions (6-15)
packages/core/src/models/SqlPrintToken.ts (1)
  • SqlPrintToken (111-141)
🔇 Additional comments (6)
docs/public/demo/index.html (1)

31-37: Copy tweak LGTM

The broadened description reads clearly and fits the demo’s scope.

docs/guide/formatting-recipes.md (1)

34-41: Verify API documentation completeness — SqlFormatterOptions.md not found

The search for API documentation did not locate SqlFormatterOptions.md in the docs directory. The formatting-recipes.md file correctly documents insertColumnsOneLine at the specified lines, with clear descriptions and usage examples. To fully verify the review, confirm whether:

  1. The API documentation file exists under a different path or name, or
  2. API documentation is not required and formatting-recipes.md is the authoritative reference

If adding a one-line example showing the interaction when valuesCommaBreak is set to 'after' would be helpful, that can be considered as an optional enhancement.

docs/public/demo/analysis-features.js (1)

64-66: __selectQueryType is a documented discriminator property, not fragile

The __selectQueryType property is explicitly defined in the SelectQuery interface as readonly __selectQueryType: 'SelectQuery'; with a comment "Discriminator property for type safety". This discriminator pattern is used throughout the codebase (e.g., in CommentEditor.ts, which checks both presence and value: '__selectQueryType' in component && (component as SelectQuery).__selectQueryType === 'SelectQuery').

The implementation is stable and consistent across SimpleSelectQuery, BinarySelectQuery, and ValuesQuery classes, all of which define the property with the literal value 'SelectQuery'. This is an intentional, documented pattern—not a fragile workaround.

To strengthen robustness, the check could verify the exact value (e.g., ast.__selectQueryType === 'SelectQuery') rather than just truthiness.

packages/core/tests/SqlFormatter.insert.test.ts (1)

79-112: LGTM: single‑line column list scenario is covered

Good coverage for insertColumnsOneLine with before‑style VALUES comma breaks.

packages/core/src/transformers/OnelineFormattingHelper.ts (1)

100-129: LGTM: insert‑columns oneline trimming and spacing are well‑scoped

Behavior is gated to InsertClause, trims leading whitespace safely, and ensures spacing via callback.

packages/core/src/transformers/SqlPrinter.ts (1)

1097-1098: LGTM: nested printers retain insertColumnsOneLine

Propagating insertColumnsOneLine into CTE/oneline printers keeps behavior consistent across modes.

Comment on lines +78 to 79
InsertClause = "InsertClause",
UpdateQuery = "UpdateQuery",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

New SqlPrintTokenContainerType.InsertClause — ensure printer/visitors handle it

Looks good; please verify all switch/if branches that act on containerType account for InsertClause (SqlPrinter, OnelineFormattingHelper, token parser).

Expected: InsertClause appears in SqlPrintTokenParser, SqlPrinter, and any oneline helpers.


🏁 Script executed:

#!/bin/bash
rg -nP "containerType\\s*[:=]|switch\\s*\\(.*containerType.*\\)" packages/core/src --type=ts -C2
rg -n "InsertClause" packages/core/src --type=ts -C2

Length of output: 24864


🏁 Script executed:

#!/bin/bash
# Find all switch statements on containerType to verify complete case coverage
echo "=== shouldFormatContainer switch ===" 
rg -n "shouldFormatContainer" packages/core/src/transformers/OnelineFormattingHelper.ts -A 15

echo ""
echo "=== isClauseLevelContainer switch ===" 
rg -n "isClauseLevelContainer" packages/core/src/parsers/SqlPrintTokenParser.ts -A 20

echo ""
echo "=== All switch statements checking containerType ===" 
ast-grep --pattern $'switch ($_) {
  $$$
}'  | grep -A 30 "containerType"

Length of output: 198976


🏁 Script executed:

#!/bin/bash
# Check indentIncrementContainers to see if InsertClause needs to be added
rg -n "indentIncrementContainers" packages/core/src/transformers/SqlPrinter.ts -B 2 -A 5

Length of output: 1961


🏁 Script executed:

#!/bin/bash
# Check where isClauseLevelContainer is called and what it affects
echo "=== isClauseLevelContainer usage ===" 
rg -n "isClauseLevelContainer|shouldAddSeparatorSpace" packages/core/src --type=ts -B 2 -A 2

echo ""
echo "=== InsertClause comment-related code ===" 
rg -n "InsertClause.*comment|comment.*InsertClause" packages/core/src --type=ts -B 2 -A 2

Length of output: 1719


Add InsertClause to isClauseLevelContainer switch for proper comment spacing

The new InsertClause enum member is handled explicitly in SqlPrinter and OnelineFormattingHelper, but is missing from the isClauseLevelContainer switch in SqlPrintTokenParser. This causes InsertClause containers to not receive proper separator spacing after comments during parsing.

Update packages/core/src/parsers/SqlPrintTokenParser.ts lines 887-902 to include InsertClause:

private isClauseLevelContainer(containerType: SqlPrintTokenContainerType): boolean {
    switch (containerType) {
        case SqlPrintTokenContainerType.SelectClause:
        case SqlPrintTokenContainerType.FromClause:
        case SqlPrintTokenContainerType.WhereClause:
        case SqlPrintTokenContainerType.GroupByClause:
        case SqlPrintTokenContainerType.HavingClause:
        case SqlPrintTokenContainerType.OrderByClause:
        case SqlPrintTokenContainerType.LimitClause:
        case SqlPrintTokenContainerType.OffsetClause:
        case SqlPrintTokenContainerType.WithClause:
        case SqlPrintTokenContainerType.SimpleSelectQuery:
        case SqlPrintTokenContainerType.InsertClause:
            return true;
        default:
            return false;
    }
}
🤖 Prompt for AI Agents
In packages/core/src/parsers/SqlPrintTokenParser.ts around lines 887 to 902, the
isClauseLevelContainer switch is missing SqlPrintTokenContainerType.InsertClause
so InsertClause containers don't get proper separator spacing after comments;
update the switch to include a case for SqlPrintTokenContainerType.InsertClause
that returns true (i.e., add "case SqlPrintTokenContainerType.InsertClause:"
alongside the other clause-level cases) so InsertClause is treated as a
clause-level container.

@mk3008 mk3008 merged commit ec6b51e into main Oct 26, 2025
7 of 8 checks passed
@mk3008 mk3008 deleted the insert_query_format branch October 26, 2025 08:17
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/core/src/transformers/SqlPrinter.ts (2)

426-435: Consider minor optimization for ensureTrailingSpace

The current logic adds a single space when missing, then normalizes all trailing whitespace. When exactly one space was just added, the regex becomes a no-op but still executes.

Consider this more efficient structure:

 private ensureTrailingSpace(): void {
     const currentLine = this.linePrinter.getCurrentLine();
     if (currentLine.text === '') {
         return;
     }
     if (!currentLine.text.endsWith(' ')) {
         currentLine.text += ' ';
+    } else if (/\s{2,}$/.test(currentLine.text)) {
+        currentLine.text = currentLine.text.replace(/\s+$/, ' ');
     }
-    currentLine.text = currentLine.text.replace(/\s+$/, ' ');
 }

1129-1177: Consider extracting statement-specific logic

The shouldConvertSpaceToClauseBreak method correctly handles various SQL statement types, but at 48 lines it's becoming complex. While functional, consider extracting statement-specific logic into separate helper methods for better maintainability:

  • shouldBreakInUpdateQuery
  • shouldBreakInInsertQuery
  • shouldBreakInDeleteQuery
  • shouldBreakInMergeAction

This would improve readability and make future extensions easier to test in isolation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 025c85a and afe21f3.

📒 Files selected for processing (1)
  • packages/core/src/transformers/SqlPrinter.ts (15 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/core/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

packages/core/**/*.{ts,tsx}: Always compile and fix TypeScript errors before running tests
Source code comments must be written in English only
For model-driven mappings (mapping.typeInfo and mapping.structure), use convertModelDrivenMapping
Use imports from 'rawsql-ts' rather than local relative paths for library usage

Files:

  • packages/core/src/transformers/SqlPrinter.ts
packages/core/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/core/AGENTS.md)

Remove console.log statements before commit

Files:

  • packages/core/src/transformers/SqlPrinter.ts
🧬 Code graph analysis (1)
packages/core/src/transformers/SqlPrinter.ts (2)
packages/core/src/transformers/OnelineFormattingHelper.ts (3)
  • CommaBreakStyle (4-4)
  • OnelineFormattingHelper (22-130)
  • OnelineFormattingOptions (6-15)
packages/core/src/models/SqlPrintToken.ts (1)
  • SqlPrintToken (111-141)
🔇 Additional comments (4)
packages/core/src/transformers/SqlPrinter.ts (4)

5-17: LGTM: Clean integration of OnelineFormattingHelper

The import structure and type aliasing are well-organized. The alias for CommaBreakStyle provides a clean public API while delegating to the helper implementation.


113-164: LGTM: Consistent property initialization

The new insertColumnsOneLine option and onelineHelper are properly declared and initialized following the existing patterns for other oneline formatting options.


437-455: LGTM: Clean delegation pattern

The tryAppendInsertClauseTokenText method provides a clear interface for INSERT clause text normalization, properly delegating to the helper and indicating success via boolean return.


457-521: LGTM: Proper comma formatting for INSERT clauses

The comma handling correctly delegates to onelineHelper.resolveCommaBreak and implements appropriate alignment for INSERT column lists. The level + 1 indentation for comma-prefixed entries and trailing space for oneline mode both follow good formatting practices.

Comment on lines +1112 to +1115
private getClauseBreakIndentLevel(parentType: SqlPrintTokenContainerType | undefined, level: number): number {
// Currently all CUD clause breaks align with the statement keyword.
return level;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify if this method is needed

The getClauseBreakIndentLevel method is defined but never called within the file. This appears to be dead code that should either be:

  • Removed if not needed
  • Integrated into the logic if it was intended for use

Run this script to confirm no usage in the codebase:


🏁 Script executed:

#!/bin/bash
# Search for any calls to getClauseBreakIndentLevel
rg -n 'getClauseBreakIndentLevel\s*\(' --type=ts

Length of output: 164



---

🏁 Script executed:

```shell
#!/bin/bash
# Broader search for getClauseBreakIndentLevel anywhere in the codebase
rg -n 'getClauseBreakIndentLevel' --type=ts --type=tsx

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Check context around the method definition (20 lines around line 1112)
rg -n 'getClauseBreakIndentLevel' -B5 -A15 packages/core/src/transformers/SqlPrinter.ts

Length of output: 1012


🏁 Script executed:

#!/bin/bash
# Search for any reference to the method, including in comments or other files
rg 'getClauseBreakIndentLevel' --type=ts --type=tsx --type=js

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Check if method might be called via object reference or dynamic calls
rg -n 'this\.getClauseBreakIndentLevel' packages/core/src/transformers/SqlPrinter.ts

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Look for the method in test files too
fd -e spec.ts -e test.ts | xargs rg -l 'getClauseBreakIndentLevel' 2>/dev/null || echo "No test files reference this method"

Length of output: 35


Remove unused getClauseBreakIndentLevel method

This private method at lines 1112-1115 is never called anywhere in the codebase. The method takes parameters but only returns level unchanged, with no implementation of the stated comment about CUD clause breaks. Remove this dead code.

🤖 Prompt for AI Agents
In packages/core/src/transformers/SqlPrinter.ts around lines 1112 to 1115,
remove the unused private method getClauseBreakIndentLevel(parentType:
SqlPrintTokenContainerType | undefined, level: number): number { return level; }
because it is never referenced and provides no behavior; delete the method
declaration and any associated unused imports or types if they become
unreferenced after removal, then run the project typecheck/tests to ensure no
references remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant