Repository navigation
fix(codegen): find generated classes by name, and derive emitted envelope bases from the bundle - #1705
Open
bokelley wants to merge 1 commit into
Open
fix(codegen): find generated classes by name, and derive emitted envelope bases from the bundle#1705bokelley wants to merge 1 commit into
bokelley wants to merge 1 commit into
Conversation
…lope bases from the bundle scripts/post_generate_fixes.py matched 29 exact class-signature strings such as "class ListCreativesResponse(AdcpVersionEnvelope, ProtocolEnvelope):". adcp#8547 makes core/protocol-envelope.json compose core/version-envelope.json and every task response compose only the protocol envelope, so the generated classes become "class X(ProtocolEnvelope):"; one pass then raised "ListCreativesResponse marker missing" and the status-alias, verify-brand-claim and compatibility-enum passes would have skipped silently (#1680). Every pass that needs a generated class now finds it by name (find_class_header / require_class_header) and keeps whatever bases the generator wrote; a class that is gone is a RuntimeError naming the pass. The classes this script emits itself (response arms, the constructible compatibility bases, the verify_brand_claim modules) take their envelope bases from response_envelope_bases(), which reads whether the generated ProtocolEnvelope already composes AdcpVersionEnvelope, and drop the version-envelope import when nothing uses it. The identity-match constant anchors on either envelope import. Measured both ways: the current 3.3.0-beta.1 tree regenerates byte-for-byte (make validate-generated), and against adcp#8547's schemas the post-generation passes run to completion and emit "class X(AdcpResponse, ProtocolEnvelope):" throughout, where they crashed before. Closes #1680 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean build-time codegen tooling fix with no blocking or medium findings.
What I checked:
- Only 2 files changed:
scripts/post_generate_fixes.py(+142/-72) and a new 165-line testtests/test_post_generate_envelope_matchers.py. No edits tosrc/adcp/types/domains/**,_generated.py,aliases.py, or any public-surface export — so semver, forward-compat, and generated-code-is-not-source gates do not apply. - The change replaces 29 brittle exact class-signature string matches with name-based
find_class_header/require_class_headerlookups and derives envelope bases from the generatedProtocolEnvelope. This makes the status-alias, verify-brand-claim, and compatibility-enum passes robust to an adcp#8547 base-list change instead of silently skipping — a fail-closed improvement (missing class now raises a named RuntimeError rather than silently no-op). - Output is byte-for-byte identical on the current 3.3.0-beta.1 tree; no CI gate disabled, no test silenced, no credential/ctx_metadata or import-layering concern. New tests cover both pre- and post-8547 compositions.
Gate checks: high_risk=false; gated_paths=false; no no-auto-approve team match; no prior decision. No critical/high/medium findings. Falls through decision table to row 9.
This branch has not been deployed
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
Closes #1680.
scripts/post_generate_fixes.pymatched 29 exact class-signature strings (class X(AdcpVersionEnvelope, ProtocolEnvelope):). adcp#8547 changes the generated response classes toclass X(ProtocolEnvelope):, which made one pass raiseListCreativesResponse marker missingand would have let the status-alias, verify-brand-claim and compatibility-enum passes skip silently.find_class_header/require_class_header) and keeps whatever bases the generator wrote; a missing class is aRuntimeErrornaming the pass.verify_brand_claim(s)modules) take their envelope bases fromresponse_envelope_bases(), which reads whether the generatedProtocolEnvelopealready composesAdcpVersionEnvelope, and drop the version-envelope import when nothing uses it. The identity-match constant anchors on either envelope import.tests/test_post_generate_envelope_matchers.py(11 tests) exercises the passes under both compositions and guards against a full-signature marker reappearing.Measured
make validate-generatedpasses (byte-for-byte),tests/test_code_generation.pyand the no-explicit-null rewrite tests pass.class X(AdcpResponse, ProtocolEnvelope):for 117 response arms plus(AdcpResponse, ResponseArmDispatchMixin, ProtocolEnvelope)for the five constructible bases, where the run crashed before. The status-alias and ListCreatives XOR passes apply. That run still stops later, in export consolidation, on a circular import the new schema set exposes (core/creative_assets.pyevaluatesasset_union.AssetVariantat class definition whileasset_unionis mid-import); that belongs to the Generated response types can't represent the protocol envelope #1678 regeneration and is written up there.🤖 Generated with Claude Code