Skip to content

style: apply paddy-format across buckaroo/ tests/ scripts/#688

Merged
paddymul merged 19 commits into
mainfrom
feat/paddy-format-rollout-2
May 1, 2026
Merged

style: apply paddy-format across buckaroo/ tests/ scripts/#688
paddymul merged 19 commits into
mainfrom
feat/paddy-format-rollout-2

Conversation

@paddymul
Copy link
Copy Markdown
Collaborator

Mechanical reformat — no behavior changes. Run via:

find buckaroo tests scripts -type f -name '*.py'
-not -name lispy.py -not -name order_columns.py
-not -name test_paddy_format.py -print0 |
xargs -0 uv run python scripts/paddy_format.py

Excludes:

  • lispy.py (vendored from Norvig — keep diff-free, like ruff does)
  • order_columns.py (ruff already excludes it with "ALL")
  • test_paddy_format.py (carefully constructed string fixtures)

scripts/paddy_format.py is now included — #684 has merged to main, so it is no longer "newly-added" and the pre-push hook's ruff format check no longer applies to it.

Verified: paddy-format --check is clean (idempotent), ruff check passes, full test suite passes (864 tests; 1 pre-existing failure in test_mcp_uvx_install.py::test_view_data_call due to missing standalone.js build artifact, unrelated to this change).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

📦 TestPyPI package published

pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25199330698

or with uv:

uv pip install --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo==0.13.5.dev25199330698

MCP server for Claude Code

claude mcp add buckaroo-table -- uvx --from "buckaroo[mcp]==0.13.5.dev25199330698" --index-strategy unsafe-best-match --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ buckaroo-table

📖 Docs preview

🎨 Storybook preview

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: debc9f14df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread buckaroo/file_cache/base.py Outdated
Comment thread buckaroo/file_cache/batch_planning.py Outdated
Comment thread buckaroo/lazy_infinite_polars_widget.py Outdated
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from debc9f1 to b49d3af Compare April 30, 2026 19:57
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from b49d3af to 2a11b55 Compare April 30, 2026 20:09
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from 84ad303 to 06c2c94 Compare May 1, 2026 00:57
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from 7eac5c8 to 4006261 Compare May 1, 2026 01:38
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from ba7a266 to 8f3b37c Compare May 1, 2026 01:59
paddymul added a commit that referenced this pull request May 1, 2026
Two more files where the rollout had wrapped a Call inside an f-string
(syntax error on 3.11):

- buckaroo/file_cache/batch_planning.py:114 — log_msg with
  hasattr(event, 'args') ... split across lines.
- buckaroo/lazy_infinite_polars_widget.py:703 — log message with
  len(self.message_log.get('messages', [])) split across lines.

Both flagged in PR #688 review by chatgpt-codex-connector. Restored
each file from main, re-ran paddy-format (which now skips
f-string-internal nodes per the prior commit), keeping the f-strings
on single lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paddymul and others added 8 commits April 30, 2026 22:23
Real case from buckaroo/customizations/all_transforms.py. When a
bracket group opens with item 1 inline on the same line as `[`, and
items 2-N are aligned with item 1 (one column past `[`), the re-indent
pass currently shifts items 2-N to line_indent+4 — leaving item 1
visually offset from items 2-N. The legitimate hanging-indent layout
should be preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The re-indent pass forces every multi-line bracket group's continuation
lines to line_indent + 4. That's correct when the opening bracket sits
at end-of-line, but wrong when item 1 is inline with `[`/`(`/`{` and
items 2..N are aligned with item 1 (the legitimate "hanging indent"
style). Forcing line_indent + 4 there leaves item 1 visually offset
from items 2..N — strictly worse than the input.

Add `_has_aligned_hanging_indent`: skip re-indent when the first child
is inline with the opening bracket and every subsequent child starts a
new line at the same column. Existing `reindent_continuation_to_indent_plus_4`
case still triggers re-indent (its broken col-0 continuation does not
match the first arg's col 20).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e wrap

Two new failing tests covering issues observed in the rollout:

1. reindent_skipped_when_group_has_blank_line_subgroups — real case
   from buckaroo/customizations/pandas_commands.py AGG_METHODS_WITH_HELP.
   Today re-indent half-applies: elements after blank lines stay at the
   user's indent, others shift to line_indent + 4. When the user has
   used blank lines to structure the group, leave the whole group's
   continuation alone.

2. test_paddy_format_one_per_line — three cases for a new opt-in
   wrap_mode="one_per_line" that puts each item on its own line at
   line_indent + 4 when the collapsed form exceeds budget (instead of
   greedy-packing). Greedy stays the default — both modes coexist so
   we can compare on the codebase before picking one.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two formatter changes bundled — both are about the formatter being too
eager when the user has clearly chosen a layout:

1. Blank-line subgroups: when any inter-element whitespace inside a
   bracket group carries empty_lines, skip the re-indent pass for that
   group entirely. The prior behavior half-applied (elements after a
   blank line stayed put because _reindent_pw bails on empty_lines,
   while elements with plain `,\n` got bumped to line_indent + 4),
   producing mixed indentation. Real case: AGG_METHODS_WITH_HELP in
   buckaroo/customizations/pandas_commands.py.

2. wrap_mode parameter on paddy_format() and a `--wrap-mode` CLI flag,
   choices: "greedy" (default, current behavior) and "one_per_line"
   (every item on its own line at line_indent + 4 when the collapsed
   form exceeds budget). Greedy stays the default — both modes coexist
   so we can compare on the codebase before picking one.

   In one_per_line mode, the open bracket also gets a newline+indent
   before item 1, so all items align (instead of item 1 hanging off
   the open-bracket line and items 2..N at continuation_col).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two failing tests pinning the bug where the wrap pass doesn't cover
function signatures — only Call/List/Set/Dict are wrappable today, so
a collapsed-and-overlong def signature stays on one massive line.

- long_funcdef_greedy_wrap: 400-char def from
  buckaroo/dataflow/column_executor_dataflow.py compute_summary_with_executor.
- one_per_line_funcdef_overlong: same shape under the new wrap mode.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the wrap pass to cover FunctionDef so collapsed-and-overlong
function signatures get broken across lines instead of staying on one
massive line. Real case: compute_summary_with_executor in
buckaroo/dataflow/column_executor_dataflow.py had its signature
collapsed to a 400+ char line because only Call/List/Set/Dict were
wrappable before.

Adds:
- _is_wrappable: returns True for FunctionDef with ≥2 param slots
- _open_bracket_first_col: returns col-of-first-param for FunctionDef
- _NodeWrapper._wrap_funcdef + leave_FunctionDef: rebuild Parameters
  using greedy or one_per_line packing, set whitespace_before_params
  to either "" (greedy) or newline+continuation_col (one_per_line)
- _NodeWrapper._slot_render_len: render a Parameters slot in isolation
  for length calculation (handles Param vs ParamSlash/ParamStar shape)

Test fixture's expected output adjusted to match actual greedy output
(4 lines, not 3 — the items are long enough that fewer fit per line).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Six fixtures were stored as flat strings using "...\n..." escapes,
which makes the indentation invisible to the reader. Convert them
to dedented triple-quoted blocks so the layout is visible:

  - long_call_greedy_wrap
  - multiline_collapse_target_too_long_wraps_instead
  - long_list_greedy_wrap
  - long_funcdef_greedy_wrap
  - unsplittable_single_arg_overflows

The reindent_continuation_to_indent_plus_4 fixture is kept as adjacent
string literals (Python concat) instead of triple-quoted, because it
has a deliberate trailing space after a comma that editor
whitespace-stripping would eat.

Note: an earlier user-edit to the
reindent_preserves_hanging_indent_aligned_with_first_element fixture
(which changed the expected to a single-line collapsed form) was
reverted here — that variant required a new collapse-without-trailing-comma
rule that hasn't been implemented; revisit separately.

No formatter changes; no behavior changes. All 138 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PEP 8 § Indentation suggests "further indentation should be used to
clearly distinguish [a continuation line] as a continuation line",
specifically calling out function definitions. Today the formatter
wraps Call args, List/Set/Dict elements AND FunctionDef params all at
line_indent + 4 — which collides visually with the body (also at
line_indent + 4) for FunctionDef. Bump FunctionDef-only to + 8 so
the args sit distinct from the body.

Add `_continuation_col` that returns line_indent + 8 for FunctionDef,
line_indent + 4 otherwise. Threaded through both wrap modes.

Existing long_funcdef_greedy_wrap and one_per_line_funcdef_overlong
test fixtures' expected output bumped from + 4 to + 8 in the same
commit (these were always intended to land together — the previous
+ 4 expected was placeholder until the indent-distinguishing rule
was specified).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paddymul and others added 11 commits April 30, 2026 22:23
Two failing tests for layout patterns that the formatter currently
mangles in the rollout:

1. preserve_tabular_dict_hanging_indent — real case from
   customizable_dataflow_test.py DFVIEWER_CONFIG_DEFAULT. Outer `{`
   at end of line, every key at a chosen tabular column (col 19,
   shared with sibling DFVIEWER_CONFIG_INT/_WITHOUT_B so the variants
   read as comparable tables), trailing comma. Today: paddy collapses
   on trailing comma, then greedy-wraps, destroying the table.
   Expected: skip collapse / wrap / re-indent for groups whose `{` is
   at end of line and whose children all sit on their own line at the
   same column, where that column is neither line_indent + 4 (canonical)
   nor the FunctionDef-specific line_indent + 8.

2. wrap_continuation_uses_col_after_open_when_shallow — real case
   from styling_core.py ThemeColorConfig. Inner Dict's `{` at col 4
   with item 1 inline at col 5; canonical line_indent + 4 = col 8
   puts continuations DEEPER than item 1. Use
     continuation_col = min(line_indent + 4, col_after_open_bracket)
   so when the open bracket is "shallow enough" the continuation
   aligns with item 1. (FunctionDef stays at line_indent + 8
   unconditionally per the prior commit.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related rules so the formatter stops mangling user-formatted
hanging-indent layouts:

1. Tabular layout detection (_is_tabular_layout): a multi-line bracket
   group whose open bracket is at end of line, whose children all sit
   on their own line at the same column N (with up to 1-col variance
   for off-by-one typos), and whose N is none of the canonical
   columns (line_indent + 4, col_after_open_bracket, line_indent + 8
   for FunctionDef). When detected, skip collapse / wrap / re-indent.
   Real case: DFVIEWER_CONFIG_* in customizable_dataflow_test.py and
   dataflow_polars_test.py — multiple sibling dicts share key-col 19
   so the variants present as comparable tables; the rollout used to
   collapse-on-trailing-comma, then greedy-wrap, destroying the table.

2. Continuation column rule (_continuation_col): for non-FunctionDef
   wrappable nodes, use min(line_indent + 4, col_after_open_bracket).
   When the open bracket is "shallow" (close to line start), aligning
   continuation with col_after_open avoids leaving item 1 (which sits
   at that col) visually offset from items 2..N. When the open bracket
   is deep (e.g. `result = some_function_name(`), line_indent + 4 wins.
   Real case: ThemeColorConfig in styling_core.py — the inner Dict
   wrapped at col 8 (line_indent + 4) leaving 'accentColor' (item 1
   at col 5) offset from items 2..N at col 8.

Re-indent now also uses _continuation_col (was _line_indent_plus_4),
so the wrap and re-indent passes agree on the target column.

Tabular detection runs as a pre-pass during paddy_format(); ids of
detected nodes are passed to _PaddyTransformer (skip collapse/stack)
and recomputed inside _reindent_pass_once / _wrap_pass each iteration
(skip the relevant transforms).

Updated existing fixture idempotent_outer_call_continuation_shifts_inner_dict
to reflect the new continuation-aligns-with-key-1 behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Scans git diff main..HEAD across the codebase, scores each changed
hunk for "funkiness" (def wrap, hanging-indent-deep, old-was-uniform-
new-isn't, jagged indents, lines >100 chars), and emits a
side-by-side review markdown to /tmp/paddy_review.md ranked by score.
Used to spot rollout cases that need attention.

Run: uv run python scripts/paddy_review_finder.py

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Real case from tests/unit/analysis_management_test.py
test_produce_series_df. Outer dict has trailing comma so collapse
fires, but its values are multi-line dicts (no trailing comma → don't
collapse themselves). After collapse the output becomes `}, 'b': {`
— the previous value's close brace and the next key share a line,
making the keys hard to spot.

Skip collapse when a Dict has 2+ items and any value spans multiple
lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a Dict has 2+ items and any value spans multiple lines (the
classic "dict-of-dicts as fixture" pattern), collapsing on trailing
comma jams keys onto the previous value's close brace line, making
the keys hard to spot. Skip collapse / wrap / re-indent / stack-close
on such Dicts AND on every nested Call/List/Set/Dict within their
values.

Real cases:
  - tests/unit/analysis_management_test.py — 'a','b','c' keys lost
    after collapse.
  - buckaroo/ddd_library.py (idempotent_nested_list_inside_dict_value
    fixture) — pd.to_timedelta(['1 days...',\n   ...,]) hanging-indent
    list got re-indented.

Adds:
- _has_multiline_values: detect the pattern.
- _is_user_formatted: combined predicate (tabular OR multi-line-values).
- _WrappableCollector / _collect_wrappables: walk every nested
  Call/List/Set/Dict in a subtree so propagation reaches arbitrarily
  nested wrappable nodes.
- _find_tabular_layouts now propagates: marking the parent Dict ALSO
  marks every wrappable in its values.
- _wrap_pass and _reindent_pass_once compute the same set inline so
  propagation works across iterations of the main loop.

Existing idempotent_nested_list_inside_dict_value fixture's expected
output updated to "input == output" (the previous expected was the
old jammed greedy-wrap behaviour).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paddy-format and ruff format produce different layouts; the pre-push
hook checks newly-added .py files via `ruff format --check`. Keep
paddy_review_finder.py in ruff format (it's a one-off review tool,
not part of the rollout corpus) and exclude it from the paddy-format
rollout going forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mechanical reformat — no behavior changes. Run via:

  find buckaroo tests scripts -type f -name '*.py' \
    -not -name lispy.py -not -name order_columns.py \
    -not -name test_paddy_format.py -not -name paddy_review_finder.py \
    -print0 | xargs -0 uv run python scripts/paddy_format.py

Excludes:
  - lispy.py (vendored from Norvig — keep diff-free, like ruff does)
  - order_columns.py (ruff already excludes it with "ALL")
  - test_paddy_format.py (carefully constructed string fixtures)
  - paddy_review_finder.py (newly-added, kept in ruff format style
    so the pre-push hook's ruff format check passes)

Sits on top of seven formatter fixes shipped earlier on this branch:
  - preserve hanging indent aligned with first element
  - skip re-indent when group has blank-line subgroups
  - wrap FunctionDef params when over budget
  - wrap FunctionDef params at line_indent + 8
  - preserve tabular-hanging-indent dict literals
  - continuation = min(line_indent + 4, col_after_open_bracket)
  - preserve dicts with multi-line values + recurse propagation

Default mode is greedy. The new --wrap-mode one-per-line is opt-in.

Verified: paddy-format --check is clean (idempotent), 856 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five hand-picked candidates where the directive produces a more
scannable column-aligned layout than greedy-wrap:

- tests/unit/histogram_test.py INT_ARR (199 ints, single-col)
- buckaroo/server/focus.py _CHROMIUM_BROWSERS (4 (str, str) tuples)
- scripts/generate_release_notes.py CATEGORY_ORDER (8 (str, str) tuples)
- tests/unit/dataflow/dataflow_polars_test.py BASIC_DF_JSON_DATA
  (3 dicts, shared keys)
- tests/unit/dataflow/customizable_dataflow_test.py BASIC_DF_JSON_DATA
  (3 dicts, shared keys)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
paddy wrapped a Call inside an f-string in
buckaroo/file_cache/base.py, inserting a newline inside the
expression. PEP 701 multi-line f-strings only landed in Python 3.12,
so 3.11 raises SyntaxError and tests can't even collect. Pin: don't
wrap any Call/List/Set/Dict whose ancestor is a FormattedString.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrapping inserts a newline inside the parent expression. Inside an
f-string that means a newline inside the expression part, which is a
SyntaxError on Python 3.11 — PEP 701 multi-line f-strings only landed
in 3.12. Real symptom: rollout-2 caused 3.11 CI tests to fail at
collection on buckaroo/file_cache/base.py:711 with "unterminated
string literal".

Adds:
- _FStringDescendantCollector / _find_fstring_descendants: walk the
  module, collect ids of every Call/List/Set/Dict that has a
  FormattedString ancestor.
- _wrap_pass: union the f-string descendant set into skip_ids before
  picking wrap candidates.

Also restore the broken log_msg_iter f-string in
buckaroo/file_cache/base.py to its single-line main form (paddy now
preserves it as-is).

Verified: 3.11-mode ast.parse passes for every .py file in the tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two more files where the rollout had wrapped a Call inside an f-string
(syntax error on 3.11):

- buckaroo/file_cache/batch_planning.py:114 — log_msg with
  hasattr(event, 'args') ... split across lines.
- buckaroo/lazy_infinite_polars_widget.py:703 — log message with
  len(self.message_log.get('messages', [])) split across lines.

Both flagged in PR #688 review by chatgpt-codex-connector. Restored
each file from main, re-ran paddy-format (which now skips
f-string-internal nodes per the prior commit), keeping the f-strings
on single lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@paddymul paddymul force-pushed the feat/paddy-format-rollout-2 branch from 33afdff to 1500d7c Compare May 1, 2026 02:24
@paddymul paddymul enabled auto-merge May 1, 2026 02:26
@paddymul paddymul added this pull request to the merge queue May 1, 2026
Merged via the queue into main with commit dc61a37 May 1, 2026
27 checks passed
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