Rewrite #148
Open
rayakame wants to merge 18 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Rewrite of the plugin internals into a config/transform/render/driver pipeline, reaching full feature parity with the previous generator and adding new features on top: - PostgreSQL enum support: enum columns generate enums.py with str-based enum classes, schema-qualified naming and runtime value coercion - new model_type pydantic (pydantic.BaseModel, requires pydantic >= 2.9) - query_parameter_limit is now implemented (default 4): queries above the limit bundle their parameters into a <Query>Params class - docstring generation ported for google/numpy/pep257 incl. embedded SQL - type overrides (db_type/column incl. wildcards), omit_unused_models, emit_init_file and sqlite adapter/converter registration restored - model and row class fields now escape reserved names (id -> id_) - fixes: :one return annotations are Optional, enum/copyfrom/kwargs handling, sqlite type resolution during table matching, import placement Test matrix extended to 8 outputs per driver (pydantic classes/functions), new pydantic pytest suites and enum runtime tests; full nox pipeline green (1261 tests). README documents enums, pydantic and the sqlite PARSE_DECLTYPES requirement; changie fragments added for v0.5.0.
|
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:
📝 WalkthroughWalkthroughThe generator was rewritten around configuration, transformation, driver, rendering, and writer packages. It adds PostgreSQL enum modules, Pydantic models, query parameter limits, reserved-name escaping, structured logging, and expanded asyncpg, aiosqlite, and sqlite3 validation coverage. ChangesGenerator pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Comment |
The asyncpg_check CI job compares committed files against raw plugin output via sqlc diff, but ruff format was reformatting the generated enums.py files (like models.py/queries.py, which were already excluded before enums existed). Exclude enums.py as well and restore the eight committed enums.py fixtures to raw generator output.
The formatter now runs on all generated files instead of excluding them: - line-length raised to 320 (ruff's maximum) - statements that would exceed it are emitted exploded, one argument per line with magic trailing commas (signatures, decode hooks, execute calls, copyfrom records/columns), so ruff format leaves them unchanged - long sqlite parameter tuples are hoisted into a local sql_args variable - blank line after module docstrings and around nested defs, single-line single-element __all__, (x,) instead of (x, ) - matching ruff's layout The *_check CI jobs (sqlc diff) and ruff format now agree on the exact same bytes: committed fixtures are raw generator output and the formatter verifies them as already clean.
Generated query functions now use regular positional-or-keyword parameters by default. The keyword-only marker is only emitted when omit_kwargs_limit is explicitly set to a non-negative value (queries with more parameters than the limit become keyword-only); unset or negative values disable the enforcement entirely. The attrs test matrix entries opt in with omit_kwargs_limit: 0 to keep the keyword-only path covered in CI. PLR0917 joins the generated-code lint ignores since query functions mirror their SQL parameter count.
…gs_limit The opt-in change in the previous commit targeted the wrong option. omit_kwargs_limit is back to its original behavior: default 0, every query with parameters is keyword-only, negative values are rejected. query_parameter_limit is now the opt-in option: when explicitly set to a non-negative value, queries with more parameters than the limit take a single params: <Query>Params argument; unset or negative values never bundle (matching the previous releases, so no longer a breaking change). :copyfrom keeps its Params class regardless.
Correctness fixes from the PR review: - table matching required only ONE column to match instead of ALL, silently mis-mapping query results onto model classes; now every column must match by name, type, and source table (and the query column types are precomputed instead of rebuilt per candidate table) - duplicate column/parameter names are deduplicated with numeric suffixes (name, name_2, ...) instead of generating Python syntax errors - enum constant names are sanitized (non-alphanumerics to underscores, VALUE_N fallback for empty, suffixes for case-duplicates) and enum values are escaped in generated string literals - nullable convertible scalar returns get a None guard; list columns with convertible element types (enum/bytea arrays, overrides) convert element-wise via comprehensions; nullable scalar :many element types are now QueryResults[T | None] end to end - ModelImports scans enum/list usage in a dedicated pass instead of a side effect inside an early-returning closure - queryValueUses scans all columns including embeds and aggregates runtime-vs-TYPE_CHECKING across occurrences instead of deciding from the first match; identical import lines from different specs are deduplicated - models/enums imports are per-module based on actual references, not global flags, so multi-file projects no longer get unused imports - inflection_exclude_table_names matches again (singularization now runs on the raw snake_case name before camel-casing) and enum type names are no longer singularized
:many functions now return QueryResults(...) instead of QueryResults[T](...). The subscripted form routed every call through typing's _GenericAlias.__call__, which additionally tried to set __orig_class__ on the instance and swallowed the resulting AttributeError each time (QueryResults uses __slots__, so the attribute was never stored anyway) — measured at roughly 10x the cost of a plain constructor call with no runtime or type-checking benefit, since the function's return annotation already carries QueryResults[T]. The import resolver no longer forces scalar :many element types to runtime imports (that existed only because the subscription evaluated the element type at runtime), so those imports return to the TYPE_CHECKING block. Verified with pyright strict on the regenerated packages: no loss of caller-side typing.
sqlite3.go and aiosqlite.go were character-identical apart from await wrapping and the cursor annotation; both drivers are now a single sqliteBase implementation parameterized by module name and an async flag (connection/cursor types are derived from the module name, and a small stmt helper produces the three await-wrapping shapes). The sqlite conversion catalog previously lived in three hand-synced structures (SQL-name set, Python-name order slice, and a spec map whose converter keys re-listed the SQL names); adding a type while missing one of them silently skipped adapter emission. It is now a single ordered spec slice from which membership, emission order, and register_converter keys are all derived. Pure refactor: regenerated output is byte-identical (empty fixture diff), full nox and all sqlc diff check sessions green.
- drop the write-only QueryValue.DBName field (only Column.DBName is consumed, by :copyfrom) and the dead Name assignments on Returns - delete the commented-out debug block in Handler - stop checking the error from strings.Builder.WriteString (documented to always be nil), dropping the writer package's log dependency - exclude the internal IR types (model.PyType/QueryValue/Column, render.importSpec, plugin.Identifier) from exhaustruct and shrink the transform literals to only meaningful fields No output change: regenerated fixtures are byte-identical.
Owner
Author
|
@coderabbitai configuration |
|
Configuration used: defaults CodeRabbit configuration# Source: defaults
language: en-US
# Source: defaults
tone_instructions: ''
# Source: defaults
early_access: false
# Source: defaults
enable_free_tier: true
# Source: defaults
inheritance: false
reviews:
# Source: defaults
profile: chill
# Source: defaults
request_changes_workflow: false
# Source: defaults
high_level_summary: true
# Source: defaults
high_level_summary_instructions: ''
# Source: defaults
high_level_summary_placeholder: '`@coderabbitai` summary'
# Source: defaults
high_level_summary_in_walkthrough: false
# Source: defaults
auto_title_placeholder: '`@coderabbitai`'
# Source: defaults
auto_title_instructions: ''
# Source: defaults
review_status: true
# Source: defaults
review_details: false
# Source: defaults
review_progress: true
# Source: defaults
commit_status: true
# Source: defaults
fail_commit_status: false
# Source: defaults
collapse_walkthrough: true
# Source: defaults
changed_files_summary: true
# Source: defaults
sequence_diagrams: true
# Source: defaults
estimate_code_review_effort: true
# Source: defaults
assess_linked_issues: true
# Source: defaults
related_issues: true
# Source: defaults
related_prs: true
# Source: defaults
suggested_labels: true
# Source: defaults
labeling_instructions: []
# Source: defaults
mutually_exclusive_groups: {}
# Source: defaults
auto_apply_labels: false
# Source: defaults
suggested_reviewers: true
# Source: defaults
auto_assign_reviewers: false
# Source: defaults
suggested_reviewers_instructions: []
# Source: defaults
in_progress_fortune: true
# Source: defaults
poem: false
# Source: defaults
enable_prompt_for_ai_agents: true
# Source: defaults
path_filters: []
# Source: defaults
path_instructions: []
# Source: defaults
abort_on_close: true
# Source: defaults
disable_cache: false
slop_detection:
# Source: defaults
enabled: true
auto_review:
# Source: defaults
enabled: true
# Source: defaults
description_keyword: ''
# Source: defaults
auto_incremental_review: true
# Source: defaults
auto_pause_after_reviewed_commits: 5
# Source: defaults
ignore_title_keywords: []
# Source: defaults
labels: []
# Source: defaults
drafts: false
# Source: defaults
base_branches: []
# Source: defaults
ignore_usernames: []
finishing_touches:
docstrings:
# Source: defaults
enabled: true
unit_tests:
# Source: defaults
enabled: true
simplify:
# Source: defaults
enabled: false
autofix:
# Source: defaults
enabled: true
fix_ci:
# Source: defaults
enabled: true
resolve_merge_conflict:
# Source: defaults
enabled: true
# Source: defaults
custom: []
pre_merge_checks:
# Source: defaults
override_requested_reviewers_only: false
docstrings:
# Source: defaults
mode: warning
# Source: defaults
threshold: 80
title:
# Source: defaults
mode: warning
# Source: defaults
requirements: ''
description:
# Source: defaults
mode: warning
issue_assessment:
# Source: defaults
mode: warning
# Source: defaults
custom_checks: []
# Source: defaults
post_merge_actions: []
tools:
ast-grep:
# Source: defaults
rule_dirs: []
# Source: defaults
util_dirs: []
# Source: defaults
essential_rules: true
# Source: defaults
packages: []
shellcheck:
# Source: defaults
enabled: true
ruff:
# Source: defaults
enabled: true
markdownlint:
# Source: defaults
enabled: true
github-checks:
# Source: defaults
enabled: true
# Source: defaults
timeout_ms: 90000
languagetool:
# Source: defaults
enabled: true
# Source: defaults
enabled_rules: []
# Source: defaults
disabled_rules: []
# Source: defaults
enabled_categories: []
# Source: defaults
disabled_categories: []
# Source: defaults
enabled_only: false
# Source: defaults
level: default
biome:
# Source: defaults
enabled: true
hadolint:
# Source: defaults
enabled: true
swiftlint:
# Source: defaults
enabled: true
phpstan:
# Source: defaults
enabled: true
# Source: defaults
level: default
phpmd:
# Source: defaults
enabled: true
phpcs:
# Source: defaults
enabled: true
golangci-lint:
# Source: defaults
enabled: true
yamllint:
# Source: defaults
enabled: true
gitleaks:
# Source: defaults
enabled: true
trufflehog:
# Source: defaults
enabled: true
checkov:
# Source: defaults
enabled: true
tflint:
# Source: defaults
enabled: true
detekt:
# Source: defaults
enabled: true
eslint:
# Source: defaults
enabled: true
e18e:
# Source: defaults
enabled: true
flake8:
# Source: defaults
enabled: true
fbinfer:
# Source: defaults
enabled: true
# Source: defaults
enable_java: false
fortitudeLint:
# Source: defaults
enabled: true
rubocop:
# Source: defaults
enabled: true
buf:
# Source: defaults
enabled: true
regal:
# Source: defaults
enabled: true
actionlint:
# Source: defaults
enabled: true
zizmor:
# Source: defaults
enabled: true
pmd:
# Source: defaults
enabled: true
clang:
# Source: defaults
enabled: true
cppcheck:
# Source: defaults
enabled: true
opengrep:
# Source: defaults
enabled: true
semgrep:
# Source: defaults
enabled: true
circleci:
# Source: defaults
enabled: true
clippy:
# Source: defaults
enabled: true
sqlfluff:
# Source: defaults
enabled: true
squawk:
# Source: defaults
enabled: true
trivy:
# Source: defaults
enabled: true
prismaLint:
# Source: defaults
enabled: true
pylint:
# Source: defaults
enabled: true
oxc:
# Source: defaults
enabled: true
shopifyThemeCheck:
# Source: defaults
enabled: true
luacheck:
# Source: defaults
enabled: true
brakeman:
# Source: defaults
enabled: true
dotenvLint:
# Source: defaults
enabled: true
htmlhint:
# Source: defaults
enabled: true
stylelint:
# Source: defaults
enabled: true
checkmake:
# Source: defaults
enabled: true
osvScanner:
# Source: defaults
enabled: true
oasdiff:
# Source: defaults
enabled: true
reactDoctor:
# Source: defaults
enabled: true
presidio:
# Source: defaults
enabled: true
blinter:
# Source: defaults
enabled: true
smartyLint:
# Source: defaults
enabled: true
emberTemplateLint:
# Source: defaults
enabled: true
skillspector:
# Source: defaults
enabled: true
psscriptanalyzer:
# Source: defaults
enabled: true
chat:
# Source: defaults
art: true
# Source: defaults
allow_non_org_members: true
# Source: defaults
auto_reply: true
integrations:
jira:
# Source: defaults
usage: auto
linear:
# Source: defaults
usage: auto
knowledge_base:
# Source: defaults
opt_out: false
web_search:
# Source: defaults
enabled: true
code_guidelines:
# Source: defaults
enabled: true
# Source: defaults
filePatterns: []
learnings:
# Source: defaults
scope: auto
# Source: defaults
approval_delay: 0
issues:
# Source: defaults
scope: auto
jira:
# Source: defaults
usage: auto
# Source: defaults
project_keys: []
linear:
# Source: defaults
usage: auto
# Source: defaults
team_keys: []
pull_requests:
# Source: defaults
scope: auto
mcp:
# Source: defaults
usage: auto
# Source: defaults
disabled_servers: []
# Source: defaults
automatic_repository_linking: false
# Source: defaults
linked_repositories: []
code_generation:
docstrings:
# Source: defaults
language: en-US
# Source: defaults
path_instructions: []
unit_tests:
# Source: defaults
path_instructions: []
issue_enrichment:
auto_enrich:
# Source: defaults
enabled: false
planning:
# Source: defaults
enabled: true
auto_planning:
# Source: defaults
enabled: true
# Source: defaults
labels: []
labeling:
# Source: defaults
labeling_instructions: []
# Source: defaults
auto_apply_labels: false
|
- exclude generated sqlc fixtures and wasm binaries from reviews - disable flake8/pylint (ruff is the single source of truth) and sqlfluff (false positives on sqlc macros) - disable the unit-test finishing touch - strip the default-value comment noise
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
internal/config/config.go-123-125 (1)
123-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport SQL-driver validation errors accurately.
This wraps unknown or incompatible drivers as an “unknown model type,” obscuring the actual configuration error.
Proposed fix
if err := conf.SqlDriver.Validate(engine); err != nil { - return fmt.Errorf("invalid options: unknown model type: %w", err) + return fmt.Errorf("invalid options: invalid SQL driver: %w", err) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/config.go` around lines 123 - 125, Update the conf.SqlDriver.Validate(engine) error path to report SQL-driver validation failures with an accurate driver-validation context instead of labeling them as an unknown model type, while preserving the original error via wrapping.internal/utils/common.go-15-22 (1)
15-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a blank line before the final return
internal/utils/common.go:22This trips
nlreturn, and the repo’s lint target includes that rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/utils/common.go` around lines 15 - 22, Add a blank line between the schema defaulting logic and the final return in the table comparison function, preserving all existing comparison behavior and satisfying the repository’s nlreturn lint rule.Sources: Coding guidelines, Linters/SAST tools
internal/model/naming.go-15-54 (1)
15-54: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a builder in
SnakeToCamelandUpperSnakeCase
The repeated+=concatenation in these loops is whatperfsprintflags. Switching tostrings.Builderwill clear the lint failure without changing behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/model/naming.go` around lines 15 - 54, The loops in SnakeToCamel and UpperSnakeCase repeatedly concatenate strings with +=, triggering perfsprint. Replace these accumulators with strings.Builder, write each generated segment or rune to the builder, and preserve the existing output and digit-prefix behavior.Sources: Coding guidelines, Linters/SAST tools
internal/transform/queries.go-122-128 (1)
122-128: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the manual slice copy in
newGoEmbed. Usecopyorslices.Cloneinstead of the element-by-element loop atinternal/transform/queries.go:216-219.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/transform/queries.go` around lines 122 - 128, In newGoEmbed, replace the element-by-element slice-copy loop around the existing manual copy with the built-in copy function or slices.Clone. Preserve the copied elements, ordering, and resulting slice behavior.Sources: Coding guidelines, Linters/SAST tools
internal/model/types.go-23-31 (1)
23-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore required Go lint compliance.
Lines 31 and 98 violate
nlreturn, so the mandated lint target fails.Proposed fix
if t.IsNullable { type_ += " | None" } + return type_ @@ } } + return falseAs per coding guidelines, “Run the Go test suite with
go test -shuffle=on ./...and maintain formatting and lint compliance with the repository Make targets.”Also applies to: 89-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/model/types.go` around lines 23 - 31, Restore nlreturn compliance in PyType.Print and the additional affected function around lines 89–98 by adding the required blank lines before return statements where control-flow blocks end. Preserve the existing type-formatting behavior and run the repository’s formatting, lint, and Go test targets afterward.Sources: Coding guidelines, Linters/SAST tools
internal/log/logger.go-35-49 (1)
35-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
%vfor errors and keep the fallback structured.%erenders errors as%!e(...), and interpolating the fallback JSON string can break escaping. FormatLogErrwith%v, and have the marshal-failure path emit a structured fallback entry instead of raw JSON text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/log/logger.go` around lines 35 - 49, Update Logger.LogErr to format the supplied error with %v instead of %e. In Logger.LogAny’s json.Marshal failure path, pass a structured log value through the existing logger mechanism rather than interpolating a raw JSON string, while preserving the marshal-failure context.README.md-15-16 (1)
15-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the example plugin version to match the documented features.
The example still downloads
v0.4.5, while these lines state PostgreSQL enums and Pydantic requirev0.5.0. Users copying it will not receive the advertised functionality.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 15 - 16, Update the example plugin version referenced near the documented feature list from v0.4.5 to v0.5.0 so copied examples receive PostgreSQL enum and Pydantic support.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/prepare_release.yml:
- Line 46: Update the release-version substitution command at the sed invocation
to read the value from an environment variable rather than interpolating
steps.latest.outputs.output into shell source. Validate that the version matches
the expected release-version format before running sed, then use the validated
environment value to update PluginVersion in internal/config/constants.go.
In `@internal/driver/asyncpg.go`:
- Around line 170-177: Update the record expression generation around the
`singleComprehension` and multiline `paramParts` paths to append a trailing
comma when `len(paramParts) == 1`, ensuring single-column COPY records are
emitted as one-element tuples while preserving existing multi-column formatting.
In `@internal/driver/common.go`:
- Around line 12-14: Bring all listed Go code into lint compliance: in
internal/driver/common.go lines 12-14, run golines formatting and replace the
trivial Sprintf in writeFuncSignature; in internal/driver/asyncpg.go lines 72,
111, and 162-165, extract repeated decode expressions as needed, reuse the
shared str constant, preallocate slices, and replace trivial Sprintf calls; in
internal/driver/conversion.go lines 46 and 70, reuse the shared str and bool
constants; in internal/render/imports.go lines 127, 208, 353, and 522, extract
repeated enums, operator, and datetime literals and add the required blank line
before break; and in internal/log/logger.go lines 54-63, replace looped string
concatenation with a builder or JSON marshaling. Run the repository Make targets
to verify formatting and lint compliance.
In `@internal/driver/conversion.go`:
- Around line 120-126: Update the parameter adapter collection around the add
function in conversion logic so overridden input types are still included when
findSqliteConversion resolves their SQLite adapter. Preserve excluding
overridden types in the separate return-value collection path, and ensure
convertParamExpr’s DefaultType conversion has the required adapter for date,
datetime, decimal, and memoryview parameters.
In `@internal/driver/rowbuilder.go`:
- Around line 69-77: Move the unexported RowBuilder method
formatEmbedConstruction below all exported methods in the type, preserving its
implementation and behavior so the configured funcorder check passes.
In `@internal/driver/sqlite_base.go`:
- Around line 145-153: Extract the repeated `return [self._decode_hook(row) for
row in result]` expression into a package-level constant in the relevant driver
package, then reuse that constant in both `WriteQueryResultsAwaitFunction` and
`WriteQueryResultsCallFunction` in the SQLite generation flow and the
corresponding asyncpg flow. Preserve the generated output exactly while removing
the duplicated string literal.
In `@internal/model/naming.go`:
- Around line 119-124: The naming logic in internal/model/naming.go lines
119-124 and internal/transform/queries.go lines 162-168 must probe suffixes in a
loop until an unused identifier is found, rather than reserving one and
returning it. Update the enum-name handling and generated parameter/field
handling to avoid duplicate identifiers while preserving the existing base-name
behavior.
- Around line 15-39: Update SnakeToCamel to return a deterministic valid
non-empty Python class-name fallback when normalization produces an empty
string, while preserving existing initialism, casing, and leading-digit
behavior. Add coverage for punctuation-only identifiers through the ModelName or
EnumName naming path.
In `@internal/render/enums.go`:
- Around line 32-35: Update the enum constant rendering loop in the generated
file body to use a proper Python string-literal quoting or escaping helper,
covering newlines and all control characters in addition to backslashes and
quotes. Preserve each constant’s name and emitted assignment format while
removing the manual partial escaping.
In `@internal/render/queries.go`:
- Around line 58-60: Introduce and reuse a single Python string-literal escaping
helper. In internal/render/queries.go lines 58-60, encode the complete SQL
constant before emitting it instead of interpolating it into an unescaped
triple-quoted literal; in internal/driver/asyncpg.go lines 166 and 190-192, pass
each COPY column name plus table and schema names through the same helper before
generating Python code. Update the relevant query-rendering and COPY generation
symbols without changing their surrounding behavior.
In `@internal/transform/queries.go`:
- Around line 93-116: Update the table-matching logic in the query
transformation loop to compare complete PyType metadata, not only the Type
field, including nullability and collection semantics. Precompute or retain the
full PyType values for pluginQuery.Columns and compare each table column’s full
type metadata before reusing an existing table model.
In `@internal/types/postgresql.go`:
- Around line 83-84: Update the type-mapping switch in the PostgreSQL type
conversion logic to match both unqualified "uuid" and explicitly-qualified
"pg_catalog.uuid" values in the UUID case. Preserve the existing uuid.UUID
output so qualified UUID columns are not routed through the default
catalog-schema handling.
In `@internal/writer/docstrings.go`:
- Around line 404-414: Update the SQL emission block in the docstring writer to
escape each SQL line before passing it to WriteIndentedLine: escape backslashes
and neutralize triple-double-quote delimiters so embedded SQL cannot terminate
the Python docstring or create invalid escapes. Preserve the existing blank-line
handling and code-fence output.
In `@internal/writer/queryresults.go`:
- Line 17: Wrap the WriteQueryResultsClassHeader function signature across
multiple lines so it complies with the 130-character golines formatter limit,
preserving all parameters and behavior.
In `@internal/writer/writer.go`:
- Around line 109-112: Update CodeWriter.WriteAll to pass cmp.Compare directly
to slices.SortFunc, removing the redundant inline comparator wrapper while
preserving the existing string sorting behavior.
In `@test/driver_aiosqlite/sqlc.yaml`:
- Around line 132-173: Regenerate the Python outputs for both Pydantic
configurations in the test driver setup after rebuilding the WASM plugin, then
commit the complete generated modules under the classes and functions output
trees, including models.py, queries.py, and related files required by the new
tests.
---
Minor comments:
In `@internal/config/config.go`:
- Around line 123-125: Update the conf.SqlDriver.Validate(engine) error path to
report SQL-driver validation failures with an accurate driver-validation context
instead of labeling them as an unknown model type, while preserving the original
error via wrapping.
In `@internal/log/logger.go`:
- Around line 35-49: Update Logger.LogErr to format the supplied error with %v
instead of %e. In Logger.LogAny’s json.Marshal failure path, pass a structured
log value through the existing logger mechanism rather than interpolating a raw
JSON string, while preserving the marshal-failure context.
In `@internal/model/naming.go`:
- Around line 15-54: The loops in SnakeToCamel and UpperSnakeCase repeatedly
concatenate strings with +=, triggering perfsprint. Replace these accumulators
with strings.Builder, write each generated segment or rune to the builder, and
preserve the existing output and digit-prefix behavior.
In `@internal/model/types.go`:
- Around line 23-31: Restore nlreturn compliance in PyType.Print and the
additional affected function around lines 89–98 by adding the required blank
lines before return statements where control-flow blocks end. Preserve the
existing type-formatting behavior and run the repository’s formatting, lint, and
Go test targets afterward.
In `@internal/transform/queries.go`:
- Around line 122-128: In newGoEmbed, replace the element-by-element slice-copy
loop around the existing manual copy with the built-in copy function or
slices.Clone. Preserve the copied elements, ordering, and resulting slice
behavior.
In `@internal/utils/common.go`:
- Around line 15-22: Add a blank line between the schema defaulting logic and
the final return in the table comparison function, preserving all existing
comparison behavior and satisfying the repository’s nlreturn lint rule.
In `@README.md`:
- Around line 15-16: Update the example plugin version referenced near the
documented feature list from v0.4.5 to v0.5.0 so copied examples receive
PostgreSQL enum and Pydantic support.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2aa6480-edca-467d-bafb-f48b51c52263
⛔ Files ignored due to path filters (84)
test/driver_aiosqlite/attrs/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/attrs/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/attrs/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/attrs/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/attrs/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/attrs/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/dataclass/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/dataclass/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/dataclass/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/dataclass/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/dataclass/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/dataclass/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/msgspec/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/msgspec/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/msgspec/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/msgspec/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/msgspec/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/msgspec/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/pydantic/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/pydantic/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/pydantic/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_aiosqlite/pydantic/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/pydantic/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/pydantic/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_aiosqlite/sqlc-gen-better-python.wasmis excluded by!**/*.wasm,!**/*.wasmtest/driver_asyncpg/attrs/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/attrs/classes/enums.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/attrs/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/attrs/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/attrs/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/attrs/functions/enums.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/attrs/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/attrs/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/dataclass/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/dataclass/classes/enums.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/dataclass/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/dataclass/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/dataclass/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/dataclass/functions/enums.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/dataclass/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/dataclass/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/msgspec/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/msgspec/classes/enums.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/msgspec/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/msgspec/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/msgspec/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/msgspec/functions/enums.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/msgspec/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/msgspec/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/pydantic/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/pydantic/classes/enums.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/pydantic/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/pydantic/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_asyncpg/pydantic/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/pydantic/functions/enums.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/pydantic/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/pydantic/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_asyncpg/sqlc-gen-better-python.wasmis excluded by!**/*.wasm,!**/*.wasmtest/driver_sqlite3/attrs/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/attrs/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/attrs/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/attrs/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/attrs/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/attrs/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/dataclass/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/dataclass/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/dataclass/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/dataclass/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/dataclass/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/dataclass/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/msgspec/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/msgspec/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/msgspec/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/msgspec/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/msgspec/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/msgspec/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/pydantic/classes/__init__.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/pydantic/classes/models.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/pydantic/classes/queries.pyis excluded by!test/driver_*/*/classes/**test/driver_sqlite3/pydantic/functions/__init__.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/pydantic/functions/models.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/pydantic/functions/queries.pyis excluded by!test/driver_*/*/functions/**test/driver_sqlite3/sqlc-gen-better-python.wasmis excluded by!**/*.wasm,!**/*.wasmuv.lockis excluded by!**/*.lock
📒 Files selected for processing (116)
.changes/unreleased/Added-20260718-120000.yaml.changes/unreleased/Added-20260718-120001.yaml.changes/unreleased/Added-20260718-120002.yaml.changes/unreleased/Breaking-20260718-120003.yaml.changes/unreleased/Changed-20260718-120004.yaml.changes/unreleased/Fixed-20260718-120006.yaml.coderabbit.yaml.coverage.github/workflows/prepare_release.yml.gitignore.golangci.ymlCLAUDE.mdMakefileREADME.mdcoverage.xmlinternal/builders.gointernal/codegen/builders/docstrings.gointernal/codegen/builders/query_results.gointernal/codegen/builders/string.gointernal/codegen/common.gointernal/codegen/drivers/aiosqlite.gointernal/codegen/drivers/asyncpg.gointernal/codegen/drivers/sqlite3.gointernal/codegen/init.gointernal/codegen/queries.gointernal/codegen/tables.gointernal/config/config.gointernal/config/constants.gointernal/config/overrides.gointernal/core/Column.gointernal/core/config.gointernal/core/enums.gointernal/core/function.gointernal/core/importer.gointernal/core/models.gointernal/core/overrides.gointernal/core/utils.gointernal/driver/asyncpg.gointernal/driver/common.gointernal/driver/conversion.gointernal/driver/driver.gointernal/driver/rowbuilder.gointernal/driver/sqlite_base.gointernal/gen.gointernal/handler.gointernal/log/logger.gointernal/log/main.gointernal/model/naming.gointernal/model/reserved.gointernal/model/singular.gointernal/model/types.gointernal/render/enums.gointernal/render/imports.gointernal/render/queries.gointernal/render/renderer.gointernal/render/tables.gointernal/transform/enums.gointernal/transform/filter.gointernal/transform/queries.gointernal/transform/tables.gointernal/transform/transformer.gointernal/transform/type.gointernal/typeConversion/asyncpg.gointernal/typeConversion/common.gointernal/typeConversion/sqlite.gointernal/types/common.gointernal/types/constants.gointernal/types/postgresql.gointernal/types/sqlite.gointernal/utils/common.gointernal/utils/constants.gointernal/writer/docstrings.gointernal/writer/queryresults.gointernal/writer/writer.goplugin/main.gopyproject.tomlruff.tomlsqlc.yamltest/conftest.pytest/driver_aiosqlite/attrs/test_aiosqlite_attrs_classes.pytest/driver_aiosqlite/attrs/test_aiosqlite_attrs_functions.pytest/driver_aiosqlite/dataclass/test_aiosqlite_dataclass_classes.pytest/driver_aiosqlite/dataclass/test_aiosqlite_dataclass_functions.pytest/driver_aiosqlite/msgspec/test_aiosqlite_msgspec_classes.pytest/driver_aiosqlite/msgspec/test_aiosqlite_msgspec_functions.pytest/driver_aiosqlite/pydantic/__init__.pytest/driver_aiosqlite/pydantic/ruff.tomltest/driver_aiosqlite/pydantic/test_aiosqlite_pydantic_classes.pytest/driver_aiosqlite/pydantic/test_aiosqlite_pydantic_functions.pytest/driver_aiosqlite/sqlc.yamltest/driver_asyncpg/attrs/test_attrs_classes.pytest/driver_asyncpg/attrs/test_attrs_functions.pytest/driver_asyncpg/dataclass/test_dataclass_classes.pytest/driver_asyncpg/dataclass/test_dataclass_functions.pytest/driver_asyncpg/msgspec/test_msgspec_classes.pytest/driver_asyncpg/msgspec/test_msgspec_functions.pytest/driver_asyncpg/pydantic/__init__.pytest/driver_asyncpg/pydantic/ruff.tomltest/driver_asyncpg/pydantic/test_pydantic_classes.pytest/driver_asyncpg/pydantic/test_pydantic_functions.pytest/driver_asyncpg/queries.sqltest/driver_asyncpg/schema.sqltest/driver_asyncpg/sqlc.yamltest/driver_sqlite3/attrs/test_sqlite3_attrs_classes.pytest/driver_sqlite3/attrs/test_sqlite3_attrs_functions.pytest/driver_sqlite3/dataclass/test_sqlite3_dataclass_classes.pytest/driver_sqlite3/dataclass/test_sqlite3_dataclass_functions.pytest/driver_sqlite3/msgspec/test_sqlite3_msgspec_classes.pytest/driver_sqlite3/msgspec/test_sqlite3_msgspec_functions.pytest/driver_sqlite3/pydantic/__init__.pytest/driver_sqlite3/pydantic/ruff.tomltest/driver_sqlite3/pydantic/test_sqlite3_pydantic_classes.pytest/driver_sqlite3/pydantic/test_sqlite3_pydantic_functions.pytest/driver_sqlite3/sqlc.yamltest/queries.sqltest/schema.sql
💤 Files with no reviewable changes (24)
- internal/core/config.go
- internal/codegen/init.go
- internal/typeConversion/asyncpg.go
- internal/typeConversion/common.go
- internal/core/Column.go
- internal/core/function.go
- internal/log/main.go
- internal/codegen/builders/query_results.go
- internal/codegen/tables.go
- internal/typeConversion/sqlite.go
- internal/codegen/drivers/sqlite3.go
- internal/codegen/queries.go
- internal/core/utils.go
- internal/core/enums.go
- internal/codegen/drivers/aiosqlite.go
- internal/codegen/drivers/asyncpg.go
- internal/core/overrides.go
- internal/builders.go
- internal/codegen/builders/string.go
- internal/codegen/builders/docstrings.go
- internal/codegen/common.go
- internal/gen.go
- internal/core/importer.go
- internal/core/models.go
- prepare_release.yml: read the changie version from an env var with format validation instead of interpolating workflow output into shell - :copyfrom with a single column now emits a one-element tuple ((param.x,) instead of the bare value (param.x)) - table matching compares full type semantics (nullability, list, enum) so LEFT JOIN results get a properly nullable Row class instead of reusing the non-null model - sqlite adapter emission includes overridden PARAM types (their DefaultType conversion needs the registered adapter); overridden return columns stay excluded (converted inline) - identifier dedup probes suffixes until unused, so a literal name_2 column can no longer collide with a generated suffix - enum values escape control characters via strconv.Quote (Go escaping is a compatible subset of Python's) - pg_catalog.uuid maps to uuid.UUID like unqualified uuid - driver-validation errors no longer claim 'unknown model type'; logger uses %v instead of the float verb %e for errors - funcorder/unlambda/S1001 fixes in touched files
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Split sqlite conversion registration by direction: parameters register adapters, non-overridden returns register converters. Previously both halves were always emitted, so an overridden parameter could install a global converter as a side effect and change what overridden return columns receive under PARSE_DECLTYPES. Derive the ciso8601 import from the emitted converter bodies instead of per-type heuristics, and import enums for overridden enum parameters that convert back via enums.X(...) at runtime. Cleanups: shared model.DedupName for identifier dedup, simplified the decimal-precision fallback, and WriteConversionSetup now consumes the ordered conversion specs directly.
…needs them Annotation-site imports now key off ConvertsInline instead of NeedsConversion, and the runtime imports for registered conversions (datetime, decimal) are derived from the same usage data that drives WriteConversionSetup. A speedups module whose only datetime usage is a converter body no longer force-imports datetime at runtime - the converter references only ciso8601, so datetime belongs in the TYPE_CHECKING block.
…sions New query modules exercise the adapter-only path (overridden decimal parameter plus datetime parameter), the converter-only path (non-overridden datetime return, with ciso8601 under speedups), and an overridden enum parameter that must still import and call the generated enum class. Runtime round-trips run in the sqlite3 dataclass and asyncpg msgspec suites.
Em-dashes, en-dashes, ellipses, arrows, box-drawing separators, curly quotes and check-mark emojis become their plain keyboard equivalents in comments, docs, changelog fragments, workflows and build scripts. Non-ASCII in .sql files is actively dangerous: sqlc rewrites parameter placeholders by byte offset, and a multi-byte character in a nearby comment silently dropped a placeholder from a query. Intentional unicode test data (the Ola mundo fixture strings) is untouched. The wasm is byte-identical (comments only), so no fixtures change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build/build.sh`:
- Line 7: Update the TARGET_DIRS array in the build script to use
whitespace-separated Bash array elements without commas, and replace the
backslash in the aiosqlite directory with a forward slash so the loop resolves
each driver directory and patches its sqlc.yaml file correctly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3bac663b-844e-49ac-8731-0ce5e3f7bc6f
📒 Files selected for processing (23)
.changes/unreleased/Added-20260718-120000.yaml.changes/unreleased/Breaking-20260718-120003.yaml.changes/unreleased/Fixed-20260718-120006.yaml.changes/unreleased/Fixed-20260719-090000.yaml.github/workflows/ci.yml.github/workflows/fragments-check.yml.github/workflows/prepare_release.ymlCLAUDE.mdinternal/config/overrides.gointernal/driver/asyncpg.gointernal/driver/common.gointernal/driver/conversion.gointernal/driver/sqlite_base.gointernal/model/naming.gointernal/render/imports.gointernal/transform/queries.goscripts/build/build.batscripts/build/build.shtest/driver_aiosqlite/schema.sqltest/driver_asyncpg/schema.sqltest/driver_sqlite3/schema.sqltest/queries.sqltest/schema.sql
🚧 Files skipped from review as they are similar to previous changes (15)
- .github/workflows/prepare_release.yml
- .changes/unreleased/Fixed-20260718-120006.yaml
- internal/driver/common.go
- test/schema.sql
- internal/config/overrides.go
- .changes/unreleased/Added-20260718-120000.yaml
- test/queries.sql
- test/driver_sqlite3/schema.sql
- internal/model/naming.go
- internal/driver/sqlite_base.go
- internal/driver/conversion.go
- test/driver_asyncpg/schema.sql
- internal/transform/queries.go
- internal/driver/asyncpg.go
- internal/render/imports.go
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 by CodeRabbit
enums.pystring-based Python enums (schema-qualified naming and runtime value coercion).model_type: pydanticfor generatingpydantic.BaseModelmodels (requirespydantic >= 2.9).query_parameter_limitto accept a single bundledparamsargument when set to a non-negative value.id→id_).:manywrapper performance by avoiding runtime generic overhead.pydantic, andquery_parameter_limit.