fix(codegen): rewrite compact ConfigDict extra in place#935
Merged
Conversation
`_set_class_extra_allow` only matched the multi-line `model_config = ConfigDict(\n ...)` form. A compact single-line `ConfigDict(extra='forbid')` fell through to the else branch, which prepended a second `model_config` block — two declarations in one class, breaking Pydantic at import time. Match the compact single-line form and flip `extra='forbid'`/`'ignore'` to `extra='allow'` in place (inserting `extra='allow'` when other args or no args are present), so exactly one `model_config` is emitted. Addresses the compact-ConfigDict residual of #907. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Clean fix. The compact and multi-line forms are now mutually exclusive arms over the same model_config, so exactly one declaration survives — which is the whole point.
Things I checked
- No multi-line mis-fire.
compact_patterngroup 2 is[^\n]*?— it can't cross a newline, so a trueConfigDict(\n ... \n)(open paren immediately followed by\n) never matches it. Andcompact_matchis only consumed in theelif, reachable only whenconfig_match is None. The multi-line path always wins when present.scripts/post_generate_fixes.py:516-517,:542. - In-place rewrite, single
model_config. Reconstruction splicesbody[:start] + group(1) + new_args + group(3) + body[end:]— rewrite in place, no prepended second block.:556-563. The new test'supdated.count("model_config") == 1locks it. - Branch coverage.
extra='allow'present →\"already\";forbid/ignore→re.subcount=1; other args →\"extra='allow', \" + args; empty →extra='allow'. All four produce valid Python (ConfigDict(extra='allow', populate_by_name=True),ConfigDict(extra='allow')).:545-554. - Quote handling.
extra=(['\"])(?:forbid|ignore)\1matches single or double quotes; replacement normalizes to single-quoteextra='allow', same as the existing multi-line arm at:521-527. Consistent. - Scope.
bodyis bounded to one class by the(?=^class |\Z)lookahead, so.searchonly sees the target class's own config. No cross-class bleed. - Layering. Build-time codegen post-processor — no
src/adcp/public surface, no wire types, nogenerated_poc/hand-edits.code-reviewer: clean, zero findings (Blocker/Major/Minor all none). Commit prefixfix(codegen)is the right semver signal — non-breaking, no public export touched.
Minor nits (non-blocking)
- Live test run not executed here. Sandbox blocked
pytest/branch checkout, so the rewrite is verified by static analysis of the regex + reconstruction, not a live run. The author reports 14/14 pluslint/typecheck/validate-generatedgreen; CI is the backstop. No concern — noting it for honesty.
LGTM.
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
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
Fixes the one remaining gap in #907:
_set_class_extra_allowinscripts/post_generate_fixes.pyonly matched the multi-linemodel_config = ConfigDict(\n ...)form. A compact single-linemodel_config = ConfigDict(extra='forbid')did not match, so it fellthrough to the else branch that prepends a fresh
model_configblock —producing two
model_configdeclarations in the same class, which breaksPydantic at import time.
The other #907 sub-items (missing
ConfigDictimport injection, enum-baseexclusion for the title-less root sentinel, the
#902short-circuit note,docs follow-ups) are already shipped. This PR closes the compact-ConfigDict
residual only.
Repro (before fix)
A compact
ConfigDict(extra='forbid')on a class that should getextra='allow'produced a duplicate:Change
_set_class_extra_allownow matches the compact single-line form(
ConfigDict(<args>)where the open paren is not followed by a newline) andrewrites it in place:
extra='allow'already present → returns"already", no changeextra='forbid'/'ignore'→ flipped toextra='allow'in placeextra=→extra='allow'inserted into the callextra='allow'insertedExactly one
model_configis emitted; the multi-line and no-config paths areunchanged.
Tests
Added
test_compact_configdict_forbid_rewritten_in_placetotests/test_extra_policy.pyasserting the compactextra='forbid'→extra='allow'rewrite and that no duplicatemodel_configis produced.Verification
uv run --extra dev pytest tests/test_extra_policy.py -q→ 14 passedmake lint→ passedmake typecheck→ passedmake validate-generated→ passed🤖 Generated with Claude Code