Add dbt, gitlab, kickstarter, and mattmc3 formatters#2
Merged
Conversation
Adds a _kw() method to the base Formatter class that controls keyword casing. All deparse methods now use self._kw() for SQL keywords, enabling formatters to output lowercase (dbt) or uppercase (river, mozilla, aweber) consistently. The dbt formatter subclasses MozillaFormatter with: - All keywords and function names lowercase - Blank lines between query clauses (generous whitespace) - CTE style: `with` on its own line, blank lines around CTE body - Explicit join types (inner join, not join) - 4-space indent, trailing commas, one item per line Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitLab: Mozilla-like with 2-space indent, uppercase keywords, blank lines inside CTE bodies, ON indented under JOIN keyword. Kickstarter: Mozilla-like with 2-space indent, uppercase keywords, JOIN ON on same line with AND indented below, compact CTE chaining (), name AS () on closing paren line. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lowercase river-style with leading commas. Keywords right-aligned to form a river, with `, item` prefix notation for continuation items in SELECT, VALUES, and SET clauses. Uses plain `join` instead of `inner join`, all keywords lowercase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CLI and library usage, all 7 style examples with sample output, and supported statement types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CLI reference: arguments, --check mode, exit codes, examples - Python API: pgfmt.format() usage, style selection, error handling - Style reference: all 7 styles with example output - Updated index with quick start and style comparison table - Removed supported statements section from README Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds four new formatter styles (dbt, gitlab, kickstarter, mattmc3), a keyword-casing hook in the base formatter, extends format dispatch and CLI choices to expose the styles, updates river/aweber CTE/width handling, and adds documentation plus parametrized tests with style-specific fixtures. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
tests/test_dbt.py (1)
7-30: Cover the new lowercase DDL/DML paths too.The added dbt fixtures here appear to be
SELECT-only, but the_kw()rollout insrc/pgfmt/formatter.pyandsrc/pgfmt/mozilla.pyalso changed INSERT/UPDATE/DELETE/CREATE formatting. Please add at least one non-SELECTfixture for a lowercase style; otherwise mixed-case regressions in those paths can ship untested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_dbt.py` around lines 7 - 30, Add at least one lowercase DDL/DML fixture to the dbt test set and update test_dbt_format/_fixture_cases to include it: create a fixture .sql in the fixtures/dbt directory containing a lowercase INSERT/UPDATE/DELETE or CREATE statement (not just SELECT), add the corresponding .expected file, and ensure _fixture_cases yields that pair so pgfmt.format(sql, style='dbt') exercises the lowercase DDL/DML paths (the recent _kw() changes in formatter.py and mozilla.py).src/pgfmt/gitlab.py (1)
325-354: Unusedclosevariable - same pattern as KickstarterFormatter.The
closevariable is initialized to empty string and never modified. Consider inlining or removing it, or adding a comment if reserved for future use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/gitlab.py` around lines 325 - 354, In _deparse_sub_link remove the unused local variable close (or document it if intentionally reserved); update all return f-strings in _deparse_sub_link to drop the trailing {close} interpolation (e.g., change f'(\n{inner}\n{close})' to f'(\n{inner}\n)') and similarly for the EXISTS_SUBLINK, ANY_SUBLINK and ALL_SUBLINK branches, or alternatively replace the close declaration with a clear comment explaining its future purpose.src/pgfmt/kickstarter.py (2)
91-106: Unnecessarycontinuestatement.The
continueon line 105 is redundant since it's the last statement in theelsebranch and the loop would naturally proceed to the next iteration.Suggested fix
else: self._format_join_item( kw, table, quals, using, lines, ) - continue self._format_item_clauses(quals, using, lines)Wait - actually looking more closely, the
continueis intentional to skip_format_item_clausesfor JOIN items since_format_join_itemhandles the clauses internally. However, this control flow is confusing.Consider restructuring to make the control flow clearer:
for kw, table, quals, using in from_items: if kw == ',': lines[-1] += ',' lines.append(f'{INDENT}{table}') + self._format_item_clauses(quals, using, lines) elif kw == from_kw: lines.append(f'{from_kw} {table}') + self._format_item_clauses(quals, using, lines) else: self._format_join_item( kw, table, quals, using, lines, ) - continue - self._format_item_clauses(quals, using, lines)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/kickstarter.py` around lines 91 - 106, The loop in the method iterates over from_items and uses a confusing continue to avoid calling _format_item_clauses after handling JOINs in _format_join_item; change the control flow in the loop so it's explicit: detect JOIN keywords (the values that trigger _format_join_item) and call self._format_join_item(kw, table, quals, using, lines) and then skip calling self._format_item_clauses, otherwise handle the ',' and from_kw branches (updating lines) and then call self._format_item_clauses(quals, using, lines); remove the redundant continue and make the branch that handles joins explicitly separate so it's clear that _format_item_clauses is not invoked for JOIN items (refer to the loop and the helper methods _format_join_item and _format_item_clauses).
348-351: Unusedclosevariable.The
closevariable is initialized to an empty string on line 348 and used in f-strings but never assigned a different value. If this is intentional (for future use or consistency), consider adding a comment. Otherwise, you can inline the empty string or remove it.Suggested fix (if close will always be empty)
- close = '' match sub_type: case 'EXISTS_SUBLINK': - return f'{self._kw("EXISTS")} (\n{inner}\n{close})' + return f'{self._kw("EXISTS")} (\n{inner}\n)' case 'ANY_SUBLINK': test = self.deparse(node.get('testexpr')) op = self._get_operator(node.get('operName', [])) if op == '=': - return f'{test} {self._kw("IN")} (\n{inner}\n{close})' - return f'{test} {op} {self._kw("ANY")} (\n{inner}\n{close})' + return f'{test} {self._kw("IN")} (\n{inner}\n)' + return f'{test} {op} {self._kw("ANY")} (\n{inner}\n)' case 'ALL_SUBLINK': test = self.deparse(node.get('testexpr')) op = self._get_operator(node.get('operName', [])) - return f'{test} {op} {self._kw("ALL")} (\n{inner}\n{close})' + return f'{test} {op} {self._kw("ALL")} (\n{inner}\n)' case _: - return f'(\n{inner}\n{close})' + return f'(\n{inner}\n)'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/kickstarter.py` around lines 348 - 351, The variable `close` in kickstarter.py is defined as an empty string and never reassigned but is interpolated into the f-string returned for the 'EXISTS_SUBLINK' branch; remove the unused `close` variable and inline the empty string (or delete it and update the f-string to not reference `close`) in the match case for sub_type 'EXISTS_SUBLINK' inside the function handling sublinks to eliminate the dead variable, or if it's intended for future use add a clarifying comment above `close` explaining why it remains an empty string.src/pgfmt/mattmc3.py (1)
116-118: Consider usingenumeratestart parameter or_for unused index.The variable
_idxsuggests it's unused, but it's actually used on line 132. Consider renaming toidxsince it's used, or restructure to avoid the underscore prefix convention for unused variables.Suggested fix
- for _idx, (kw, table, quals, using, is_qual) in enumerate( + for idx, (kw, table, quals, using, is_qual) in enumerate( from_items, ): kw_lower = kw.lower() if kw != ',' else kw if not is_qual: self._format_river_from_item( kw, kw_lower, table, quals, using, width, content_pad, lines, ) else: - if _idx > 1: + if idx > 1: lines.append('')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/mattmc3.py` around lines 116 - 118, The loop currently declares an index as `_idx` in the enumerate(...) tuple but that variable is actually used later; rename `_idx` to `idx` (or otherwise remove the leading underscore) in the for loop that unpacks `(kw, table, quals, using, is_qual)` so the index is clearly used — update every reference to `_idx` within the surrounding function in src/pgfmt/mattmc3.py (the enumerate(...) loop and the usage at the later line) to `idx`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 17: The .gitignore entry "guides/" is too broad and can hide tracked
documentation source files; update the ignore rule to only exclude generated
artifacts (e.g., specific build/output folders or file patterns) or add negation
rules for source content so authors' documentation changes are not silently
ignored—modify the "guides/" line to a narrower pattern (for example target
"guides/build/" or "guides/**/*.html") or add "!guides/**/src/"-style negations
to preserve source files while still ignoring generated outputs.
In `@src/pgfmt/dbt.py`:
- Around line 77-84: The code currently omits trailing commas: update the
SELECT-target formatting so every target line ends with a comma per the
docstring/dbt style; specifically, in the single-target branch (where it emits
f'{select_kw} {distinct}{targets[0]}') append a comma, and in the multi-target
loop change the suffix computation (currently suffix = ',' if i < len(targets) -
1 else '') to always set suffix = ',' so the last item also gets a trailing
comma; keep existing prefixes (distinct and INDENT) intact.
In `@src/pgfmt/formatter.py`:
- Around line 46-48: Several formatter methods still emit hard-coded uppercase
keywords instead of using the central casing hook; update
format_create_table_as(), format_create_function(), _format_create_domain(), and
_format_create_user_mapping() to call self._kw(keyword) for each SQL keyword
(e.g., "CREATE", "TABLE", "AS", "FUNCTION", "DOMAIN", "USER", "MAPPING", etc.)
rather than embedding literal uppercase strings, and ensure any surrounding
punctuation/spaces are preserved when replacing the hard-coded keywords so
lowercase styles consistently apply.
In `@src/pgfmt/gitlab.py`:
- Around line 365-426: Remove the redundant overrides _format_where,
_format_having, and _format_on from the GitLabFormatter class so it uses the
parent MozillaFormatter implementations; locate the methods named _format_where,
_format_having, and _format_on in src/pgfmt/gitlab.py and delete their entire
method bodies (including their signatures) so inheritance handles formatting,
and verify there are no GitLab-specific references that require custom behavior
(e.g., calls to _flatten_bool_expr or _kw) before committing.
In `@src/pgfmt/mozilla.py`:
- Around line 237-241: format_create_table() currently only uses self._kw() for
the initial CREATE prefix, leaving subsequent DDL tokens like WITH, SERVER and
OPTIONS hard-coded and causing mixed-case output for Mozilla-derived lowercase
styles; update the branches that emit those tokens (the blocks that append
"WITH", "SERVER", "OPTIONS" and any storage option keys) to call
self._kw('WITH'), self._kw('SERVER'), self._kw('OPTIONS') (and any other DDL
keywords emitted in that method) and ensure any keyword strings are routed
through _kw() so the entire CREATE TABLE / CREATE FOREIGN TABLE output follows
the configured keyword casing, using the existing foreign_server flag and
surrounding logic to locate the correct places to change.
---
Nitpick comments:
In `@src/pgfmt/gitlab.py`:
- Around line 325-354: In _deparse_sub_link remove the unused local variable
close (or document it if intentionally reserved); update all return f-strings in
_deparse_sub_link to drop the trailing {close} interpolation (e.g., change
f'(\n{inner}\n{close})' to f'(\n{inner}\n)') and similarly for the
EXISTS_SUBLINK, ANY_SUBLINK and ALL_SUBLINK branches, or alternatively replace
the close declaration with a clear comment explaining its future purpose.
In `@src/pgfmt/kickstarter.py`:
- Around line 91-106: The loop in the method iterates over from_items and uses a
confusing continue to avoid calling _format_item_clauses after handling JOINs in
_format_join_item; change the control flow in the loop so it's explicit: detect
JOIN keywords (the values that trigger _format_join_item) and call
self._format_join_item(kw, table, quals, using, lines) and then skip calling
self._format_item_clauses, otherwise handle the ',' and from_kw branches
(updating lines) and then call self._format_item_clauses(quals, using, lines);
remove the redundant continue and make the branch that handles joins explicitly
separate so it's clear that _format_item_clauses is not invoked for JOIN items
(refer to the loop and the helper methods _format_join_item and
_format_item_clauses).
- Around line 348-351: The variable `close` in kickstarter.py is defined as an
empty string and never reassigned but is interpolated into the f-string returned
for the 'EXISTS_SUBLINK' branch; remove the unused `close` variable and inline
the empty string (or delete it and update the f-string to not reference `close`)
in the match case for sub_type 'EXISTS_SUBLINK' inside the function handling
sublinks to eliminate the dead variable, or if it's intended for future use add
a clarifying comment above `close` explaining why it remains an empty string.
In `@src/pgfmt/mattmc3.py`:
- Around line 116-118: The loop currently declares an index as `_idx` in the
enumerate(...) tuple but that variable is actually used later; rename `_idx` to
`idx` (or otherwise remove the leading underscore) in the for loop that unpacks
`(kw, table, quals, using, is_qual)` so the index is clearly used — update every
reference to `_idx` within the surrounding function in src/pgfmt/mattmc3.py (the
enumerate(...) loop and the usage at the later line) to `idx`.
In `@tests/test_dbt.py`:
- Around line 7-30: Add at least one lowercase DDL/DML fixture to the dbt test
set and update test_dbt_format/_fixture_cases to include it: create a fixture
.sql in the fixtures/dbt directory containing a lowercase INSERT/UPDATE/DELETE
or CREATE statement (not just SELECT), add the corresponding .expected file, and
ensure _fixture_cases yields that pair so pgfmt.format(sql, style='dbt')
exercises the lowercase DDL/DML paths (the recent _kw() changes in formatter.py
and mozilla.py).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5df99748-b531-4c88-92b9-8eaf75453809
📒 Files selected for processing (53)
.gitignoreREADME.mddocs/api.mddocs/cli.mddocs/index.mddocs/styles.mdmkdocs.ymlsrc/pgfmt/__init__.pysrc/pgfmt/cli.pysrc/pgfmt/dbt.pysrc/pgfmt/formatter.pysrc/pgfmt/gitlab.pysrc/pgfmt/kickstarter.pysrc/pgfmt/mattmc3.pysrc/pgfmt/mozilla.pytests/fixtures/dbt/select_cte.expectedtests/fixtures/dbt/select_cte.sqltests/fixtures/dbt/select_group_order.expectedtests/fixtures/dbt/select_group_order.sqltests/fixtures/dbt/select_join.expectedtests/fixtures/dbt/select_join.sqltests/fixtures/dbt/select_simple.expectedtests/fixtures/dbt/select_simple.sqltests/fixtures/gitlab/select_cte.expectedtests/fixtures/gitlab/select_cte.sqltests/fixtures/gitlab/select_group_order.expectedtests/fixtures/gitlab/select_group_order.sqltests/fixtures/gitlab/select_join.expectedtests/fixtures/gitlab/select_join.sqltests/fixtures/gitlab/select_simple.expectedtests/fixtures/gitlab/select_simple.sqltests/fixtures/kickstarter/select_cte.expectedtests/fixtures/kickstarter/select_cte.sqltests/fixtures/kickstarter/select_join.expectedtests/fixtures/kickstarter/select_join.sqltests/fixtures/kickstarter/select_simple.expectedtests/fixtures/kickstarter/select_simple.sqltests/fixtures/kickstarter/select_where.expectedtests/fixtures/kickstarter/select_where.sqltests/fixtures/mattmc3/insert_values.expectedtests/fixtures/mattmc3/insert_values.sqltests/fixtures/mattmc3/select_join.expectedtests/fixtures/mattmc3/select_join.sqltests/fixtures/mattmc3/select_or.expectedtests/fixtures/mattmc3/select_or.sqltests/fixtures/mattmc3/select_simple.expectedtests/fixtures/mattmc3/select_simple.sqltests/fixtures/mattmc3/update_multi.expectedtests/fixtures/mattmc3/update_multi.sqltests/test_dbt.pytests/test_gitlab.pytests/test_kickstarter.pytests/test_mattmc3.py
- Narrow .gitignore to only ignore guides/sqlfmt.html instead of the entire guides/ directory - Replace hardcoded uppercase keywords with self._kw() calls in format_create_function(), _format_create_domain(), _format_create_user_mapping(), and format_create_table_as() - Remove redundant _format_where, _format_having, and _format_on overrides in GitLabFormatter that were identical to the inherited MozillaFormatter methods - Replace hardcoded WITH, SERVER, and OPTIONS keywords with self._kw() in mozilla.py and gitlab.py format_create_table() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pgfmt/gitlab.py (1)
325-353: Remove the unusedclosevariable.The
closevariable on line 334 is initialized to an empty string and never modified. It's used in the f-strings but adds no value. Consider removing it for clarity.♻️ Suggested simplification
def _deparse_sub_link(self, node: dict) -> str: sub_type = node.get('subLinkType', '') subselect = node['subselect'] inner_node = subselect.get('SelectStmt', subselect) inner = self.format_select( inner_node, indent=len(INDENT), ) - close = '' match sub_type: case 'EXISTS_SUBLINK': - return f'{self._kw("EXISTS")} (\n{inner}\n{close})' + return f'{self._kw("EXISTS")} (\n{inner}\n)' case 'ANY_SUBLINK': test = self.deparse(node.get('testexpr')) op = self._get_operator( node.get('operName', []), ) if op == '=': - return f'{test} {self._kw("IN")} (\n{inner}\n{close})' - return f'{test} {op} {self._kw("ANY")} (\n{inner}\n{close})' + return f'{test} {self._kw("IN")} (\n{inner}\n)' + return f'{test} {op} {self._kw("ANY")} (\n{inner}\n)' case 'ALL_SUBLINK': test = self.deparse(node.get('testexpr')) op = self._get_operator( node.get('operName', []), ) - return f'{test} {op} {self._kw("ALL")} (\n{inner}\n{close})' + return f'{test} {op} {self._kw("ALL")} (\n{inner}\n)' case _: - return f'(\n{inner}\n{close})' + return f'(\n{inner}\n)'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/gitlab.py` around lines 325 - 353, In _deparse_sub_link remove the unused local variable close (declared on the line initializing close = '') and update the f-strings in the match cases to omit {close}; keep the rest of the logic identical—this affects the function _deparse_sub_link and its returned string constructions that currently include {close} alongside inner and the surrounding parentheses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pgfmt/gitlab.py`:
- Around line 325-353: In _deparse_sub_link remove the unused local variable
close (declared on the line initializing close = '') and update the f-strings in
the match cases to omit {close}; keep the rest of the logic identical—this
affects the function _deparse_sub_link and its returned string constructions
that currently include {close} alongside inner and the surrounding parentheses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f08f28c-a98c-42e4-9583-a94a7e0b9bdd
📒 Files selected for processing (3)
src/pgfmt/formatter.pysrc/pgfmt/gitlab.pysrc/pgfmt/mozilla.py
✅ Files skipped from review due to trivial changes (1)
- src/pgfmt/mozilla.py
pgparse represents zero values as empty dicts ({"ival": {}} for 0,
{"fval": {}} for 0.0), similar to FALSE booleans. Use .get() with
defaults to handle missing inner keys.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pgfmt/formatter.py (1)
734-736: Boolean values in storage options bypass_kw()casing.Unlike other boolean handling (e.g., lines 360-363), the
'true'/'false'literals here are hardcoded lowercase and don't useself._kw(). If this is intentional (PostgreSQL storage options may require lowercase), consider adding a brief comment. Otherwise, wrap with_kw()for consistency.♻️ Suggested fix if consistency is desired
elif arg and 'Boolean' in arg: - val = 'true' if arg['Boolean'].get('boolval') else 'false' + val = self._kw('TRUE') if arg['Boolean'].get('boolval') else self._kw('FALSE') parts.append(f'{name}={val}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pgfmt/formatter.py` around lines 734 - 736, The boolean storage-option branch in formatter.py constructs literal 'true'/'false' without using the formatter's keyword-casing helper; update the branch in the block handling arg with 'Boolean' so val is produced via self._kw('true') or self._kw('false') (i.e., replace the hardcoded lowercase literals with calls to self._kw()) before building parts.append(f'{name}={val}'); if lowercase is intentional, instead add a short comment above this branch explaining why casing is forced rather than using _kw().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pgfmt/formatter.py`:
- Around line 734-736: The boolean storage-option branch in formatter.py
constructs literal 'true'/'false' without using the formatter's keyword-casing
helper; update the branch in the block handling arg with 'Boolean' so val is
produced via self._kw('true') or self._kw('false') (i.e., replace the hardcoded
lowercase literals with calls to self._kw()) before building
parts.append(f'{name}={val}'); if lowercase is intentional, instead add a short
comment above this branch explaining why casing is forced rather than using
_kw().
AWeber formatter now pre-computes the max river width across all CTEs and the outer SELECT, ensuring consistent keyword alignment throughout the entire statement. Added min_width parameter to format_select and _format_with_clause to propagate the unified width. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CASE expressions now render with WHEN/ELSE aligned and END offset to match the river position. Both `CASE WHEN` and `CASE expr WHEN` forms are handled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three new fixtures derived from production queries: - Multi-CTE with UNION and final aggregated JOIN - CASE expression with multiple LEFT JOINs - Nested CTE-in-CTE with RANK() window function Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Nested CTEs (WITH inside a CTE body) now indent their WITH header right-aligned to the outer river width, with the nested body indented to the content column. The nested body uses its own river width for internal alignment. Added _format_cte_body hook to base RiverFormatter for subclass override. AWeber overrides it to detect and indent nested CTEs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a CASE expression's THEN or ELSE result is multi-line (e.g., an EXISTS subquery), continuation lines are now indented to align after the THEN/ELSE keyword using _indent_continuation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pgfmt/aweber.py`:
- Around line 12-30: In format_select, when a WITH clause is present you must
recompute the river width to account for nested CTEs that can introduce a wider
river keyword; replace the current conditional so you compute width =
max(min_width, self._unified_river_width(node)) (call _unified_river_width even
when min_width != 0) and pass that width into super().format_select (use the
withClause check and the format_select signature already present). Also apply
the same change to the other similar branches handling WITH/nested scopes in
this file so nested scopes always consider the locally computed unified river
width.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- In format_select, always compute max(min_width, _unified_river_width) when a WITH clause is present, instead of only when min_width == 0 - In _format_cte_body, compute scope_width as max(min_width, _unified_river_width) so the nested WITH block layout uses the correct alignment column even when called with an inherited width - Pass scope_width (not the inherited min_width) to the outer query format_select call inside _format_cte_body Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_kw()method to base Formatter for keyword casing (enables lowercase styles)_kw()for subclass reuseNew Styles
inner joinjoinTest plan
pgfmt --style dbtproduces lowercase output with blank linespgfmt --style mattmc3produces leading comma river output🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests