Bundle: sync surface-contract enforcement + surgical regeneration (#1900, #1938)#1960
Open
Serhan-Asad wants to merge 18 commits into
Open
Bundle: sync surface-contract enforcement + surgical regeneration (#1900, #1938)#1960Serhan-Asad wants to merge 18 commits into
Serhan-Asad wants to merge 18 commits into
Conversation
…ace contract (#1900) The public-surface gate compared OLD-vs-NEW generated code, so it could not distinguish an intended interface change from generator drift, and its repair directive had no stable target ("restore compatible signatures" — compatible with the very code being regenerated). This dead-ended `pdd change -> pdd sync` whenever a module's declared interface legitimately evolved (live case pdd_cloud#2971: an added symbol plus unrelated `list_secret_metadata` drift). Make `_verify_public_surface_regression` declaration-aware, per-symbol: - Symbols declared in the prompt's <pdd-interface> (`type: module`) are validated against the DECLARED signature (a stable target) via `signature_entries_compatible`, with defaults resolved in the generated namespace (#1558). Binding-kind/async — which <pdd-interface> cannot express — stay anchored to the prior generation so async<->sync / function<->class drift is still caught; `BREAKING-CHANGE: change signature` remains the escape hatch for those. - Undeclared symbols keep the old-code baseline (+ `BREAKING-CHANGE`), so backward-compat protection for helpers/re-exports is unchanged (no regression). - `PublicSurfaceRegressionError` carries structured `signature_detail:` lines (full expected-vs-actual, no truncation); the agentic subprocess parser recovers them into the rebuilt repair directive and hard-failure block. `removed:` / `signature_changed:` lines stay byte-identical (cloud parser + tests depend on them). Scoped to `type: module` function declarations; cli/command names and dotted `Class.__init__`/new-class presence stay owned by the conformance gate. Adds tests/test_issue_1900_surface_contract.py (18 gate-level tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…enerator_main PDD-Auto-Heal-Checkpoint: success
… / parser hardening Follow-up to the #1900 public-surface declaration contract (the preceding auto-heal bot commit for examples/architecture/metadata is left untouched): - Declared dotted names (`Class.method`, `Class.__init__`) are presence-only in the surface gate: `_snapshot_public_signatures` receiver-strips `self`/`cls`, so comparing a declared method signature that includes `self` false-positived on UNCHANGED code. Their parameter contract stays with the `<pdd-interface>` signature gate; top-level functions keep full declared-signature validation. - `BREAKING-CHANGE: change signature <sym>` now relaxes ONLY the un-declarable binding-kind/async for a declared symbol, never its declared params/return — an added-required-param that violates the declaration is no longer bypassable by prose. - `_parse_signature_detail_lines` tolerates ` | ` inside a signature/default (dropped the ordered-index precheck; right-anchored rsplit + try/except only), so union types / delimiter-like defaults keep the stable declared repair target. - Sync README + the `code_generator_main` generator prompt (5b) to the declaration-aware behavior (incl. `PublicSurfaceRegressionError.signature_details`). Adds 4 regression tests (22 total in tests/test_issue_1900_surface_contract.py). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…code baseline (#1900) Review follow-up: making dotted-method / non-paren-class declared symbols presence-only ALSO removed them from the old-code baseline, dropping the signature protection they had before #1900 — a declared method could silently gain a required arg or flip binding kind, and a declared class's constructor ABI could drift. Only symbols actually validated against the declared signature (top-level names with a parseable paren signature) are now recorded in `declared_validated` and excluded from the old-code baseline; every other declared symbol (dotted methods, `class Service`-style non-paren declarations) falls back to the previous-generation old-vs-new comparison, which catches added-required-params, binding-kind flips, and constructor-ABI drift, and still honors BREAKING-CHANGE + #1558. The receiver-strip false positive stays fixed (the old-code path compares receiver-stripped snapshot entries on both sides). README + generator prompt (5b) wording synced to match. Adds 4 regression tests (method +required, staticmethod->instance flip, class ctor drift, BREAKING-CHANGE opt-out) and strengthens the presence-only test that had encoded the over-skip. 26 tests in tests/test_issue_1900_surface_contract.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…face too (#1900) Review follow-up: dotted methods and constructors were presence-only with an old-code fallback, which left NEW methods/ctors unchecked (an added required arg passed) and kept the `pdd change → sync` dead-end for method contract changes. Make them first-class declared-contract citizens, exactly like top-level functions. `_declared_signature_to_entry` gains `strip_receiver`: a declared method/ctor signature is receiver-stripped (leading `self`/`cls` dropped) to match `_snapshot_public_signatures`; `Class.__init__` maps to the class `[class]` entry (constructor ABI); binding-kind/async come from the pre-generation entry when present (else the generated one) so a `@staticmethod`→instance flip is still caught; and `BREAKING-CHANGE: change signature` relaxes only those un-declarable facets, never the declared params. Only symbols actually compared against the declaration join `declared_validated` (keyed on the snapshot key) and are excluded from the old-code baseline; description-only / non-paren declarations stay presence-only and fall back to old-vs-new so no pre-#1900 protection is lost. Net: a new declared method/ctor with an added required arg now raises; editing a declared method contract is authorized without a `BREAKING-CHANGE` permit; the declared-`self` false positive stays fixed. README + generator prompt (5b) synced. Adds 3 tests and reworks 2 (29 in tests/test_issue_1900_surface_contract.py). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The auto-heal bot commit (3632d6a) regenerated context/agentic_sync_runner_example.py in a way that dropped the top-level AsyncSyncRunner shim (breaking other prompts' <include select="class:AsyncSyncRunner">), un-mocked _sync_one_module (so the example spawned real pdd sync subprocesses), and left project_dependencies.csv stale with trailing whitespace. The #1900 change is internal (surface gate + detail parser) and does not alter the public interfaces these examples demonstrate, so restore main's known-good generated artifacts. PR diff is now purely the #1900 source/tests/docs.
…allable, fix repair advice (#1900) Review follow-up (three real gaps in the declared-interface surface contract): - Declared `_`-prefixed helpers (e.g. agentic_common's `_extract_step_report`) were false-positived as removed: `declared_missing` checked presence against the public surface, which excludes underscore names. The declaration is authoritative (like `__all__`), so union the declared symbols actually defined in the code into the `patch_targets` fed to `_snapshot_public_surface` / `_snapshot_public_signatures` for both before and after — a present declared `_helper` is no longer flagged, a genuinely removed one still is, and its signature is now validated too. - A declared callable regenerated as a non-callable (`def f` → `f = 1` / `from pkg import f as f`) passed: `signature_entries_compatible` returns None and the symbol was still marked validated, so the old-code baseline was skipped. Now only add to `declared_validated` when the comparison is conclusive (`compat is not None`); a None result falls through to the old-code baseline, which flags the `[function]`→`[assignment]`/`[import]` break (and does not false-positive on an unchanged callable-assignment). - The hard-failure / repair advice told users to add `BREAKING-CHANGE`, which for a declared symbol only relaxes binding-kind/async, never params — a dead-end. Advice is now declaration-aware: a declared-param violation points at editing the `<pdd-interface>` declaration; the `BREAKING-CHANGE` guidance stays for undeclared/removed symbols. Adds 7 tests (36 in tests/test_issue_1900_surface_contract.py); the generator prompt's `<include>` now lists the new helpers; README deduped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ines, underscore-class ctor (#1900) Review follow-up (three edge gaps in the declared-interface surface contract): - A declared callable regenerated as a non-callable (`def f` → `f = 1` / `from pkg import f as f`) slipped through when the prompt also carried `BREAKING-CHANGE: change signature f` — the old-code fallback skipped it via `allowed_signature_changes`. Now, when the declared comparison is inconclusive (`compat is None`) and the OLD entry was a callable contract, flag the declared-callable-became-non-callable DIRECTLY, regardless of `BREAKING-CHANGE` (which authorizes parameter changes, not de-callable-ing a declared callable — use `BREAKING-CHANGE: remove` for that). An unchanged callable-assignment (`f = lambda: x`) still passes. - `signature_detail:` lines are now JSON-encoded (`json.dumps` of `{symbol, expected, actual, source}`) at both emit sites and parsed via `json.loads`, ending the recurring field corruption when a signature default contains the ` | actual: ` / ` | source: ` delimiter substring. - Declared underscore-CLASS constructor drift (`_Service.__init__`) was missed because the patch-target signature capture only built entries for top-level functions. Extracted `_class_constructor_signature` (shared with `_walk_class`) and used it so a patch-target class gets its `[class]` constructor-ABI entry; a declared underscore class / constructor is now validated like a public one. Adds 5 tests (41 in tests/test_issue_1900_surface_contract.py); the generator prompt spec notes the JSON detail-line format. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sses; sync self-hosting metadata (#1900) Review follow-up: - Declared NESTED constructors whose class path contains an underscore behaved inconsistently — `_Outer.Inner.__init__` passed silently, `Outer._Inner.__init__` was mischaracterized as `removed`. They now behave like the public control: `_symbol_exists_in_module` resolves nested class paths (not just methods), `_resolve_class_node` walks dotted segments to a `ClassDef`, and the patch-target signature capture emits the `[class]` constructor-ABI entry for a nested declared class (keyed `Outer.Inner`) — so ctor drift raises a `changed` signature with a `pdd-interface` detail. Surgical: only DECLARED/patch-target nested classes are captured; the public `_walk_class` recursion (undeclared underscore-nested) is untouched, so no protection is broadened. `test_split_validation.py` confirms the `_symbol_exists_in_module` generalization didn't regress sibling-test patch resolution. - Self-hosting metadata synced to the #1900 behavior so future self-regeneration keeps it: the `agentic_sync_runner` prompt documents the JSON `signature_detail:` parsing + declaration-aware repair; the `code_generator_main` prompt `<include>` lists the new helpers (`_class_constructor_signature`, `_resolve_class_node`, `_symbol_exists_in_module`, `_patch_target_signature_entry`); and architecture.json's `code_generator_main` + `agentic_sync_runner` descriptions carry the declared-contract / JSON-detail behavior. Adds 4 tests (45 in tests/test_issue_1900_surface_contract.py). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-fresh (#1938) pdd sync regenerated mature modules "rebirth-shaped" (full regeneration), dropping declared public symbols. PDD already ships edit-shaped generation (incremental_code_generator + code_patcher_LLM), but sync hardcoded force_incremental_flag=False, so it deferred to the deliberately-conservative diff_analyzer_LLM ("prefer complete regeneration"), which rebirthed modules on ordinary small prompt deltas and lost symbols. Make sync prefer surgical generation for mature modules, with a --fresh opt-out: - New `pdd sync --fresh` flag (default off), threaded sync -> sync_main -> sync_orchestration and the one-session generate loop. - Both sync generate call sites now pass force_incremental_flag=(not fresh): default -> surgical/edit-shaped (existing code + prompt delta -> minimal edit, declared symbols preserved); --fresh -> standard generation. - Reuses existing infra only. Safety nets preserved: new/empty modules still full-generate; undeterminable original prompt falls back to full generation; conformance repair retries still force full regeneration (#1724); the public-surface / declared-interface gate (#1900) is unchanged and remains the guarantee. - --fresh is single-module only: raises UsageError on global/agentic sync paths (mirrors --snapshot-context) instead of silently no-op'ing. Prompt sources edited in tandem with generated .py (pdd is self-hosted); sync_main's architecture.json signature aligned; README documents the flag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Prompt-declared <pdd-interface> as the public-surface contract (#1900). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Surgical (edit-shaped) regeneration for mature modules + --fresh (#1938). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) PR#1940 made `pdd sync` prefer surgical (edit-shaped) regeneration for mature modules, which needs an "original" prompt to diff the current prompt against. In the cloud sync-after-change flow the prompt edit is already committed on the branch (on-disk == HEAD) and the pre-change version is not reliably found by the local history search, so incremental resolution silently degraded to full regeneration and re-dropped declared symbols (live failure: test_repo#4232, 5 consecutive surface-gate blocks). Add a last-resort fallback in code_generator_main's original-prompt resolution: when no explicit original, no uncommitted-vs-HEAD delta, and no distinct prior version in local history resolve an original, read it from the base branch via the existing get_git_content_at_ref() helper. Base ref is configurable via PDD_SYNC_BASE_REF (default origin/main) and is only used when it differs from the current prompt. This replaces the prior degenerate fallback that set the "original" to the current HEAD content (identical prompts), which produced a no-op incremental patch that was then discarded for full regeneration anyway. When the base ref also yields nothing usable, can_attempt_incremental now stays False and an honest full regeneration runs, surfaced by a single unconditional (non-verbose) degradation notice so operators are not blind to mature-module full regens. Tests cover: base-ref resolves a distinct original -> incremental used; base-ref missing (None) -> full generation + loud notice, no crash; base-ref identical to current -> not used; PDD_SYNC_BASE_REF override honored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
Staging E2E evidence (2026-07-09, staging1, stack/e2e-staging-v2 = 288d053)Two full change→sync chains ran against real mirrors with this bundle deployed:
Net: the contract gate blocks only for real reasons, the repair loop engages, and the authorize→relabel loop completes. Surface regression class from round 1 (silent symbol drops) did not recur. |
…#1968) The sync public-surface gate correctly rejected a regeneration that drifted a declared `<pdd-interface>` signature only in annotation spelling (declared `object`, regenerated `Any`; or a broadened parameter union), but the repair retry re-emitted the identical "equivalent" spelling and never converged. Two complementary fixes, building on PR#1905's self-repair mechanism: 1. Verbatim repair directive: for a `signature_changed` / `source: pdd-interface` violation, both the in-process `PublicSurfaceRegressionError.repair_directive` and the subprocess `agentic_sync_runner._parse_public_surface_failure` reconstruction now inject the declared signature as a hard constraint ("emit exactly this signature ...", plus an explicit anti-substitution rule: keep `object` as `object`, never `Any`; do not broaden a declared union), instead of merely describing the violation. 2. Deterministic annotation reconciliation: `_reconcile_declared_annotation_drift` runs BEFORE the gate re-checks and, when the ONLY drift on a declared symbol is annotation spelling (identical param names/order/kinds/defaults), rewrites the emitted annotation(s) back to the declared text via a byte-offset splice, so the gate passes with no further generation attempt. Fail-safe: only gate-INCOMPATIBLE annotations are rewritten, structural drift is left for the gate/repair loop, compatible aliases (`Dict` vs `dict`) are never churned, and the rewrite is discarded if the result no longer parses. `PDD_SKIP_ANNOTATION_RECONCILE=1` bypasses it. Owning prompts (code_generator_main_python.prompt, agentic_sync_runner_python.prompt) updated in sync, mirroring PR#1905's structure. Tests: tests/test_issue_1968_annotation_convergence.py (9 cases) — verbatim directive on both paths, object/Any + broadened-union convergence, method drift, and regressions (added param / removed symbol / compatible alias / no declaration are left untouched). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # architecture.json
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.
Bundle: sync surface-contract enforcement + surgical regeneration
Rolls up the two sync-contract fixes plus the base-ref follow-up into one deployable branch off current
main(post-#1954 deterministic continuous sync reconcile).Constituent changes
fix(sync): prompt-declared <pdd-interface> as the public-surface contract (#1900)— treats the prompt's declared<pdd-interface>as the authoritative public-surface contract, validates declared methods/constructors/underscore symbols against it, and catches callable→non-callable regressions so a sync can no longer silently drop declared surface.fix(sync): surgical (edit-shaped) regeneration for mature modules + --fresh (#1938)— prefers surgical, edit-shaped regeneration for mature modules instead of full rebirth, with an explicit--freshescape hatch to force complete regeneration.fix(sync): base-branch original fallback for surgical regen (#1938/#1940)— when the pre-change prompt is already committed on-branch (cloud sync-after-change flow) and local history yields no distinct prior version, resolves the "original" prompt from the base branch (get_git_content_at_ref,PDD_SYNC_BASE_REFdefaulting toorigin/main) so surgical regen actually engages; otherwise falls back to an honest full regeneration surfaced by a single loud degradation notice.Closes #1900
Closes #1938
Provenance
Surfaced during the PDD GitHub App E2E hardening campaign (staging lanes test_repo#4232 / test_repo#4233), where mature modules were repeatedly rebirthed on sync and declared symbols were dropped, producing consecutive surface-gate blocks.
Validation
Local unit suites pass on the bundle branch:
test_issue_1900_surface_contract,test_code_generator_main,test_sync_orchestration,test_sync_main,test_one_session_sync,test_maintenance— 884 passed. Staging E2E evidence will be attached before merge.Constituent PRs #1905 and #1940 will be closed as superseded when this bundle merges (left open for now).
🤖 Generated with Claude Code