fix(schemas): merge full-corpus regen against committed schema - #3502
Merged
Conversation
Problem: SchemaRegistry.replace_provider_packages deleted a provider's entire versions/ tree and rewrote it from a fresh full-corpus generation with no merge against the committed prior schema. Every `devtools schema-generate` run goes through this path. Measured 2026-08-01 against the live archive: claude-code lost 722 of 944 typed leaf paths, codex's `timestamp` union narrowed from ["number","string"] back to ["string"] -- reproducing the exact 2026-07-29 incident that test_promotion_monotonicity.py already guards for the other promotion surface (promote_cluster / merge_observed_structure_schemas), which was never wired into this path. What changed: replace_provider_packages now collects every element schema already committed under this registry's own storage_root (across all existing versions, by element_kind) before deleting anything, and merges each incoming element schema into it via merge_observed_structure_schemas before writing -- the same monotonic-merge contract _merge_with_promoted_schema already applies for promote_cluster. Added _load_local_element_schema (a local-only, no-bundled-fallback schema reader, mirroring _load_local_catalog's existing guarantee) and _existing_provider_element_schemas/_merge_element_schema_with_existing as the supporting helpers. Verification: - python -m devtools test tests/unit/schemas/test_promotion_monotonicity.py -> 8 passed - python -m devtools test tests/unit/schemas/ tests/unit/core/test_runtime_registry_helpers.py -> 71 passed - python -m devtools verify --quick -> exit 0 Ref polylogue-ov5r Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe registry now loads local element schemas, merges historical provider schemas across package versions, and preserves observed structure and metadata during replacement. Regression tests cover field preservation, new fields, and non-narrowing type unions. ChangesProvider schema promotion
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffedecd3b5
ℹ️ 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".
| return candidate | ||
| from polylogue.schemas.generation.dynamic_keys import merge_observed_structure_schemas | ||
|
|
||
| merged = json_document(merge_observed_structure_schemas([json_document(existing), candidate])) |
There was a problem hiding this comment.
When a provider already has a local catalog, merge_observed_structure_schemas reconstructs nested nodes using only structural keywords such as type, properties, and items, while the subsequent overlay restores x-polylogue-* annotations only at the document root. Consequently, every regeneration strips freshly computed nested annotations such as x-polylogue-semantic-role, x-polylogue-format, x-polylogue-frequency, and x-polylogue-observed-distribution; this degrades schema explanation, auditing, and synthetic generation, all of which read those annotations from property nodes. The merge needs to retain the candidate's annotations recursively, with the existing schema as fallback.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Confirmed correct — landed after merge (checks/comment-count were clean when I merged, these posted moments later). Filed as polylogue-46kg with both this finding and the sibling element-kind-loss one; fast-follow fix in flight.
Comment on lines
+664
to
+668
| merged_element_schemas: ElementSchemaMap = { | ||
| element_kind: self._merge_element_schema_with_existing( | ||
| existing_element_schemas.get(element_kind), schema | ||
| ) | ||
| for element_kind, schema in element_schemas.items() |
There was a problem hiding this comment.
When a thinner regeneration contains no samples for a previously committed element kind, that kind is absent from element_schemas.items(), so the collected historical schema is never added to any prepared package; the code then deletes the old versions tree and saves the candidate-only catalog. For providers with adjunct elements, a corpus subset can therefore make get_element_schema(..., element_kind=<old kind>) return None, despite absence from the new window not proving that the provider stopped emitting it. Preserve unmatched historical element manifests and schemas, or require an explicit retirement path.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Confirmed correct, same disposition as the sibling comment — tracked in polylogue-46kg, fast-follow fix in flight.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@polylogue/schemas/runtime_registry.py`:
- Around line 592-609: Update _existing_provider_element_schemas to avoid
calling _load_local_element_schema inside the package/element loop, since
catalog is already in scope. Build the resolved element schema path directly
from the current package and element, then read it with _read_gzip_json_dict
while preserving the existing None handling and merge-by-element_kind behavior.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f8780ca4-0983-4a22-99d5-7e52d5709c16
📒 Files selected for processing (2)
polylogue/schemas/runtime_registry.pytests/unit/schemas/test_promotion_monotonicity.py
Comment on lines
+592
to
+609
| for package in catalog.packages: | ||
| for element in package.elements: | ||
| if element.schema_file is None: | ||
| continue | ||
| existing = self._load_local_element_schema( | ||
| provider_token, | ||
| version=package.version, | ||
| element_kind=element.element_kind, | ||
| ) | ||
| if existing is None: | ||
| continue | ||
| prior = merged_by_kind.get(element.element_kind) | ||
| merged_by_kind[element.element_kind] = ( | ||
| json_document(merge_observed_structure_schemas([json_document(prior), existing])) | ||
| if prior is not None | ||
| else existing | ||
| ) | ||
| return merged_by_kind |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Avoid re-loading the catalog for every element/version pair.
_load_local_element_schema (called at Line 596) internally calls self._load_local_catalog(provider_token) again, even though _existing_provider_element_schemas already holds catalog in scope at Line 586. For a provider with many package versions and elements, this re-reads and re-parses the same catalog JSON file once per element/version combination.
Since package and element are already resolved in this loop, build the element path directly and call _read_gzip_json_dict instead of routing through _load_local_element_schema.
♻️ Proposed refactor to skip the redundant catalog reload
- existing = self._load_local_element_schema(
- provider_token,
- version=package.version,
- element_kind=element.element_kind,
- )
+ path = self._provider_dir(provider_token) / "versions" / package.version / "elements" / element.schema_file
+ existing = _read_gzip_json_dict(path) if path.exists() else None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for package in catalog.packages: | |
| for element in package.elements: | |
| if element.schema_file is None: | |
| continue | |
| existing = self._load_local_element_schema( | |
| provider_token, | |
| version=package.version, | |
| element_kind=element.element_kind, | |
| ) | |
| if existing is None: | |
| continue | |
| prior = merged_by_kind.get(element.element_kind) | |
| merged_by_kind[element.element_kind] = ( | |
| json_document(merge_observed_structure_schemas([json_document(prior), existing])) | |
| if prior is not None | |
| else existing | |
| ) | |
| return merged_by_kind | |
| for package in catalog.packages: | |
| for element in package.elements: | |
| if element.schema_file is None: | |
| continue | |
| path = self._provider_dir(provider_token) / "versions" / package.version / "elements" / element.schema_file | |
| existing = _read_gzip_json_dict(path) if path.exists() else None | |
| if existing is None: | |
| continue | |
| prior = merged_by_kind.get(element.element_kind) | |
| merged_by_kind[element.element_kind] = ( | |
| json_document(merge_observed_structure_schemas([json_document(prior), existing])) | |
| if prior is not None | |
| else existing | |
| ) | |
| return merged_by_kind |
🤖 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 `@polylogue/schemas/runtime_registry.py` around lines 592 - 609, Update
_existing_provider_element_schemas to avoid calling _load_local_element_schema
inside the package/element loop, since catalog is already in scope. Build the
resolved element schema path directly from the current package and element, then
read it with _read_gzip_json_dict while preserving the existing None handling
and merge-by-element_kind behavior.
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
…ndings Co-Authored-By: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
…inds (#3503) ## Summary Fast-follow fix for two real regressions found by automated review on PR #3502 (merged a7a5765) minutes after it landed, both confirmed by reading the merged code (not speculative). ## Problem **P1 (annotation loss).** `merge_observed_structure_schemas` (`polylogue/schemas/generation/dynamic_keys.py:140`) merges only structural keywords (`type`/`properties`/`items`/`additionalProperties`) — its own docstring says "without retaining property history". The `_merge_element_schema_with_existing` added by #3502 restored `x-polylogue-*` annotations only at the document ROOT after merging, so every regeneration stripped freshly-computed *nested* annotations (`x-polylogue-semantic-role`, `x-polylogue-format`, `x-polylogue-frequency`, `x-polylogue-observed-distribution`) from property-level nodes. **P2 (element-kind loss).** When a thinner regeneration observes zero samples for a previously-committed element kind, that kind is absent from `element_schemas.items()` in the fresh pass, so the merge never runs for it, and the destructive versions-tree delete-and-rewrite in `replace_provider_packages` drops it. `get_element_schema(..., element_kind=<old kind>)` silently returns `None` afterward — the same destructive-loss bug class polylogue-ov5r (#3502) fixed, just narrower: whole missing element kinds rather than narrowed types within an observed kind. ## Solution - `SchemaRegistry._annotate_merged_schema_node` (`polylogue/schemas/runtime_registry.py`): a new recursive walk that reattaches `x-polylogue-*` annotations at *every* node of the structurally merged tree (properties/items/additionalProperties), matching existing/candidate/merged by path, preferring the candidate's fresh annotations with the existing package's as fallback. `_merge_element_schema_with_existing` calls it instead of a root-only restore. - `replace_provider_packages` now computes, per version, the union of existing-catalog element kinds and freshly-observed element kinds. A kind absent from the fresh pass is carried forward unmerged (its prior manifest + schema, pass-through), scoped to kinds the same version previously carried (matching `get_element_schema`/`package.element()`'s per-version lookup). A whole retired version disappearing entirely is a separate, out-of-scope loss class not addressed here. - Fixing P2 surfaced a **second bug in the same method**: it persisted the caller's original `catalog.packages` via `save_package_catalog(catalog)` instead of the carry-forward-augmented packages, so a carried element's schema file was written to disk but never listed in the saved manifest (`get_element_schema` would still return `None`). Fixed together via `dataclasses.replace(catalog, packages=...)`. - Low-priority fold-in: CodeRabbit flagged `_load_local_element_schema` re-reading/re-parsing the same catalog JSON per element/version inside `_existing_provider_element_schemas`'s loop when `catalog` is already in scope there. Split out `_read_local_element_schema_file` so that loop reads the schema file directly instead of reloading the catalog each time. ## Tests Added to `tests/unit/schemas/test_promotion_monotonicity.py`, following the `TestReplaceProviderPackagesMonotonicity` pattern from #3502: - `TestMergeElementSchemaAnnotationPreservation`: - `test_nested_annotation_survives_when_fresh_pass_recomputes_no_annotation` — existing schema has a nested property annotated `x-polylogue-semantic-role`; a subsequent regen re-observes the same property's type with no annotation; asserts the annotation survives. - `test_nested_annotation_fresh_value_wins_over_stale_existing` — the fresh regen carries a *different* annotation value for the same nested property; asserts the fresh value wins. - `TestReplaceProviderPackagesCarriesForwardUnobservedElementKinds`: - `test_kind_absent_from_fresh_observation_window_is_carried_forward` — a regen with a fresh sample set covering only element kind `message`, where the existing catalog has kinds `message` and `tool_result`; asserts `tool_result`'s schema still resolves via `get_element_schema` afterward, unmerged with fresh data. **Anti-vacuity:** I manually reverted `polylogue/schemas/runtime_registry.py` (keeping the new tests) and reran `devtools test tests/unit/schemas/test_promotion_monotonicity.py`: exactly the 3 new tests failed (2 annotation tests asserting `None == 'identifier'`/`'timestamp'`, 1 element-kind test asserting `None is not None`), while the 8 pre-existing tests stayed green. Then restored the fix and reconfirmed all 11 pass. ## Verification - `python -m devtools test tests/unit/schemas/` → `66 passed in 6.83s` - `python -m devtools verify --quick` → `exit_code: 0` (ruff format/check, mypy, render all, topology, layering, closure-matrix, schema roundtrip/versioning/promotion-audit, manifests, ci-workflows, doc-commands, docs-coverage, test-infra-currency, pytest-timeout-overrides, degrade-loudly, hash-boundary-census — all `ok`) - Anti-vacuity check described above (revert → 3 targeted failures → restore → green) ## AC matrix | Finding | Status | | --- | --- | | P1 — nested annotation loss during merge | Fixed: `_annotate_merged_schema_node` recursive reattach, tested | | P2 — whole element kinds silently dropped | Fixed: union-of-kinds carry-forward + catalog-persistence fix, tested | | Low-priority perf (`_load_local_element_schema` re-parse) | Fixed: `_read_local_element_schema_file` split-out | Ref polylogue-46kg Co-authored-by: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
Problem: across a ~28-PR merge train in one coordinator session (2026-08-01), two incidents showed operator memory doesn't scale as the sole safety net before squash-merging. PR #3502 merged with 0 review comments showing at check time; CodeRabbit posted 3 real findings 30-60s later (caught only by an ad hoc grace-period poll habit adopted afterward). PR #3517 nearly merged carrying a 43-test regression that no CI check or review comment ever flagged, since per-PR CI deliberately skips the heavy test suite (CLAUDE.md) - caught only because the coordinator happened to run the broader local suite by hand before merging that specific time. What changed: `devtools workspace merge-gate record <PR> --command "..."` runs a local verification command against a PR's current head sha and persists a receipt keyed to that exact sha under .cache/verify/merge-gate/. `merge-gate check <PR>` BLOCKs unless a fresh, exit-0 receipt exists for the PR's CURRENT head (a new push invalidates the old receipt) and no review comment's created_at is newer than the head commit's timestamp - late comments are listed explicitly rather than requiring a human to compare two timestamps by hand. This doesn't replace judgment about what a late comment means; it makes an unverified late signal impossible to merge past silently. Verification: devtools test tests/unit/devtools/test_merge_gate.py (9 passed, covering fresh/stale-sha/late-comment/closed-PR/expired- receipt cases against a faked gh subprocess). Live smoke test against open PR #3517: record + check round-tripped correctly (BLOCK before recording, OK after). devtools verify --quick exit 0.
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
CodeRabbit review on merge-gate's own PR (#3518) found real gaps: 1. record() only copied the fetched head_sha into the receipt without verifying the local checkout was actually AT that commit -- a receipt could attest to unrelated code (e.g. recording from master or a stale worktree). Now refuses (exit 2) unless `git rev-parse HEAD` matches the PR's headRefOid and the tree is clean. 2. The documented example used `devtools verify --quick`, which explicitly skips tests -- the exact profile that would have missed PR #3517's 43-test regression, this tool's own motivating incident. Fixed the example to `devtools verify`, and record() now tags a receipt with skips_tests: true when the command looks like it didn't run tests (heuristic), which check() surfaces as an advisory. 3. check() took a single comment snapshot, so a comment posted 30-60s later (the PR #3502 incident this tool exists to prevent) could still slip through if check() ran before it landed. check() now polls comments across a configurable grace window (default 3x20s) instead of one snapshot. 4. Once a comment's created_at was later than the head commit, it blocked forever with no way to mark it triaged short of an empty commit. Added `ack <PR> <comment-id> --reason "..."`, scoped to the PR's current head sha so a new push always re-requires triage. Verification: devtools test tests/unit/devtools/test_merge_gate.py -- 16 passed (added: checkout-mismatch refusal, dirty-tree refusal, skips_tests flagging, multi-round poll catching a comment that only appears on round 2, ack suppressing a late comment for its exact head sha but not a different one). devtools verify --quick exit 0.
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
Problem: across a ~28-PR merge train in one coordinator session (2026-08-01), two incidents showed operator memory doesn't scale as the sole safety net before squash-merging. PR #3502 merged with 0 review comments showing at check time; CodeRabbit posted 3 real findings 30-60s later (caught only by an ad hoc grace-period poll habit adopted afterward). PR #3517 nearly merged carrying a 43-test regression that no CI check or review comment ever flagged, since per-PR CI deliberately skips the heavy test suite (CLAUDE.md) - caught only because the coordinator happened to run the broader local suite by hand before merging that specific time. What changed: `devtools workspace merge-gate record <PR> --command "..."` runs a local verification command against a PR's current head sha and persists a receipt keyed to that exact sha under .cache/verify/merge-gate/. `merge-gate check <PR>` BLOCKs unless a fresh, exit-0 receipt exists for the PR's CURRENT head (a new push invalidates the old receipt) and no review comment's created_at is newer than the head commit's timestamp - late comments are listed explicitly rather than requiring a human to compare two timestamps by hand. This doesn't replace judgment about what a late comment means; it makes an unverified late signal impossible to merge past silently. Verification: devtools test tests/unit/devtools/test_merge_gate.py (9 passed, covering fresh/stale-sha/late-comment/closed-PR/expired- receipt cases against a faked gh subprocess). Live smoke test against open PR #3517: record + check round-tripped correctly (BLOCK before recording, OK after). devtools verify --quick exit 0.
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
CodeRabbit review on merge-gate's own PR (#3518) found real gaps: 1. record() only copied the fetched head_sha into the receipt without verifying the local checkout was actually AT that commit -- a receipt could attest to unrelated code (e.g. recording from master or a stale worktree). Now refuses (exit 2) unless `git rev-parse HEAD` matches the PR's headRefOid and the tree is clean. 2. The documented example used `devtools verify --quick`, which explicitly skips tests -- the exact profile that would have missed PR #3517's 43-test regression, this tool's own motivating incident. Fixed the example to `devtools verify`, and record() now tags a receipt with skips_tests: true when the command looks like it didn't run tests (heuristic), which check() surfaces as an advisory. 3. check() took a single comment snapshot, so a comment posted 30-60s later (the PR #3502 incident this tool exists to prevent) could still slip through if check() ran before it landed. check() now polls comments across a configurable grace window (default 3x20s) instead of one snapshot. 4. Once a comment's created_at was later than the head commit, it blocked forever with no way to mark it triaged short of an empty commit. Added `ack <PR> <comment-id> --reason "..."`, scoped to the PR's current head sha so a new push always re-requires triage. Verification: devtools test tests/unit/devtools/test_merge_gate.py -- 16 passed (added: checkout-mismatch refusal, dirty-tree refusal, skips_tests flagging, multi-round poll catching a comment that only appears on round 2, ack suppressing a late comment for its exact head sha but not a different one). devtools verify --quick exit 0.
Sinity
added a commit
that referenced
this pull request
Aug 1, 2026
…3518) ## Summary Adds `devtools workspace merge-gate`, replacing coordinator memory (grace-period comment polling, remembering to run the broader local test suite per-PR CI skips) with a check that fails closed. ## Problem Two incidents in a single ~28-PR merge-train session (2026-08-01): 1. PR #3502 was squash-merged with 0 review comments showing at check time; CodeRabbit posted 3 real findings 30-60s later. 2. PR #3517 nearly merged carrying a 43-test regression that no CI check or review comment ever flagged (per-PR CI deliberately skips the heavy test suite — see CLAUDE.md). It was caught only because the coordinator happened to run the broader local suite by hand before merging that time. A coordinator merging dozens of PRs across a few hours cannot reliably repeat either habit purely from memory every single time. ## Solution - `devtools workspace merge-gate record <PR> --command "..."` runs a local verification command against the PR's current head sha and persists a receipt keyed to that exact sha under `.cache/verify/merge-gate/`. - `devtools workspace merge-gate check <PR>` BLOCKs unless a fresh, exit-0 receipt exists for the PR's CURRENT head (a new push invalidates the old receipt) and no review comment's `created_at` is newer than the head commit's timestamp — late comments are listed explicitly instead of requiring a human to compare timestamps by hand. Does not replace judgment about what a late finding means — makes an unverified late signal impossible to merge past silently. ## Verification `devtools test tests/unit/devtools/test_merge_gate.py` — 9 passed (fresh receipt, stale-sha, late-comment, closed-PR, expired-receipt cases against a faked `gh` subprocess). Live smoke test against open PR #3517: `check` BLOCKed before recording, then `record` + `check` round-tripped to OK. `devtools verify --quick` exit 0. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a merge-gate command for pre-merge verification. * Records local verification results tied to the current change. * Checks verification freshness, change status, and recent review comments. * Supports acknowledging valid review-comment exceptions. * Provides human-readable and JSON verdicts with configurable polling and receipt age. * **Documentation** * Added usage guidance and CLI examples for the merge-gate workflow. * **Tests** * Added comprehensive coverage for verification, freshness, review comments, acknowledgements, and failure scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sinity
added a commit
that referenced
this pull request
Aug 3, 2026
## Summary Adds `devtools workspace merge <PR>`, a wrapper around `gh pr merge --squash` that structurally enforces the existing `merge-gate` record/check safety net and the merge-train terminal full-suite-verify rule, instead of leaving both as steps a coordinator has to remember to invoke. ## Problem `devtools workspace merge-gate record/check` (merge_gate.py) and the CLAUDE.md rule "run one full-suite `devtools verify --all` per merge-train session" are both real, already-shipped fixes for real 2026-08-01 incidents: - PR #3502 was squash-merged with 0 review comments visible; CodeRabbit posted 3 real findings 30-60s later. - PR #3517 nearly merged carrying a 43-test regression that no CI check and no review comment ever flagged (per-PR CI deliberately skips the heavy suite). Both fixes work, but only if a coordinator remembers to invoke them at the right moment mid-session. The fanout-operations report's incident-ledger cross-check (referenced by beads `polylogue-ct3r2` and `polylogue-t6iga`, which are duplicate filings of the same finding) found the exact pattern: fixes that became a *command* stuck; fixes that stayed a memory-triggered *rule* recurred at least once. There is no GitHub Actions hook available to attach enforcement to here -- this repo's CI is CircleCI-only, GHA is intentionally dark. The actual merge boundary in practice is a human/agent coordinator invoking `gh pr merge --squash`, so that is the wrapping point. ## Solution `devtools/merge_boundary.py`, registered as `devtools workspace merge` in `devtools/command_catalog.py`: - `merge <PR>`: refuses unless the PR is `OPEN`; if no fresh merge-gate receipt exists for the current head sha, auto-records one (running `--command`, default `devtools verify`) instead of just erroring; runs `merge-gate check` and refuses to merge on any BLOCK (stale/missing receipt, nonzero exit, unacked late review comment); applies title hygiene by stripping a doubled `(#N) (#N)` squash-subject suffix (the 2026-07-12/13 incident where a manual `gh pr edit --title` step was skipped); then runs the actual `gh pr merge --squash`. `--dry-run` runs every check without merging. `--with-verify` immediately runs and records the merge-train's terminal full-suite verify after merging. - `train-status`: reports (exit 1) any PRs merged since the last recorded full-suite verify, reading a ledger at `.cache/verify/merge-gate/merge-train-ledger.json` -- the structural stand-in for "a merge-train records the full-suite verify as its terminal ledger step." - `record-full-verify --command "devtools verify --all"`: runs and records that terminal step directly, clearing the ledger's pending list. CLAUDE.md's merge-checklist bullet ("Before squash-merging any PR") now names `workspace merge` as the command to actually use, rather than describing the two-step `merge-gate record`/`check` sequence as something the coordinator must remember to run in order. The lower-level `merge-gate record/check/ack` commands are untouched and still usable directly for ad hoc receipt inspection; `merge` is a thin, tested composition on top of them (`cmd_record`/`cmd_check` are called directly, not re-implemented). ## Verification - `devtools test tests/unit/devtools/test_merge_boundary.py tests/unit/devtools/test_merge_gate.py` -> 35 passed (decision-logic tests mock `subprocess.run`/`gh` boundaries per the existing `test_merge_gate.py` pattern: receipt fresh/stale/absent, unacked newer review comment, title-hygiene collapse/append/idempotence, dry-run never calling `gh pr merge`, ledger append/pending-PR detection, `record-full-verify` clearing pending state). - `python -m mypy devtools/merge_boundary.py tests/unit/devtools/test_merge_boundary.py` -> no issues. - `devtools verify --quick` -> exit_code 0 (ruff format/check, mypy --strict, `render all --check`, topology/layering/manifests/lab-policy gates all ok). - `devtools render topology-projection` / `devtools render devtools-reference` were run and their outputs (`docs/plans/topology-target.yaml`, `docs/devtools.md`) are included in this diff. Not run: `devtools verify --all` (full non-integration suite) -- left for the coordinator's merge-train terminal step, consistent with what this PR itself builds. Ref polylogue-ct3r2, polylogue-t6iga -- both bead filings describe the same recommendation; this PR implements it once. The coordinator should dedupe the two beads (close one as duplicate-of-the-other, or close both against this PR) rather than tracking the same follow-up twice. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
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
SchemaRegistry.replace_provider_packagesnow merges a full-corpus schemaregeneration against the committed prior schema instead of destructively
replacing it, closing the last unguarded promotion surface.
Problem
SchemaRegistry.replace_provider_packages(polylogue/schemas/runtime_registry.py:543)unconditionally deletes a provider's entire
versions/tree and rewrites itfrom a fresh full-corpus generation, with no merge against the committed
prior schema. This is the code path every
devtools schema-generate/devtools lab schema generaterun writes through(
persist_generated_provider_bundle->replace_provider_packages,polylogue/schemas/generation/workflow.py).tests/unit/schemas/test_promotion_monotonicity.pyalready documents andguards against exactly this failure mode for the other promotion surface
(
SchemaRegistry.promote_cluster/merge_observed_structure_schemas),citing a real 2026-07-29 incident where a codex/claude-code promotion
"narrowed 33 field types and dropped 173 fields." That guard was wired into
promote_clusteronly.Reproduced 2026-08-01 while executing polylogue-2qx.3 AC1 (regenerate schema
packages for every provider from the live archive). A full-corpus
devtools schema-generaterun against the live archive, diffed against thecommitted HEAD schema by JSON-Schema leaf-path type union:
.timestamp["number","string"]->["string"])None of that regenerated output was committed; it was fully reverted before
this PR, per polylogue-ov5r.
Solution
replace_provider_packagesnow:own
storage_root(across all existing versions, keyed byelement_kind) via a new_existing_provider_element_schemas, backed bya new local-only
_load_local_element_schemahelper that mirrors_load_local_catalog's existing "never silently fall back to the bundledSCHEMA_DIRtree" guarantee.via
merge_observed_structure_schemasbefore writing(
_merge_element_schema_with_existing), unioningtype/propertiesthesame way
SchemaRegistryToolingMixin._merge_with_promoted_schemaalreadydoes for
promote_cluster, and carrying forward anyx-polylogue-*annotation the fresh candidate didn't recompute.
versions/is otherwise unchanged; only the schemacontent written per package/element is now the merged result instead
of the raw candidate.
Modules touched:
polylogue/schemas/runtime_registry.py(the fix),tests/unit/schemas/test_promotion_monotonicity.py(new coverage).Acceptance criteria (from polylogue-ov5r)
merge_observed_structure_schemas(or equivalent) intoreplace_provider_packagesfollowingpromote_cluster's pattern --satisfied.
lose them -- satisfied,
test_regen_observing_a_subset_of_known_fields_does_not_lose_them.test_regen_observing_genuinely_new_fields_gains_them.test_regen_type_unions_only_grow_never_narrow(the exact codextimestampincident, reproduced againstreplace_provider_packagesdirectly).
Anti-vacuity: all three new tests exercise
SchemaRegistry.replace_provider_packages-- the production entry pointpersist_generated_provider_bundle(and thereforedevtools schema-generate) calls -- by invoking it twice against the sameprovider/version with a narrower second schema and asserting the merged
result on disk. Deleting the
_merge_element_schema_with_existing/_existing_provider_element_schemascall (or revertingreplace_provider_packagesto writepackage_schemasdirectly) makes allthree fail: the second call would then simply overwrite the first package.
Out of scope for this PR (deliberately): actually regenerating/promoting any
real provider schema package. That is polylogue-2qx.3's remaining AC1 work,
to be re-run once this lands.
Verification
python -m devtools test tests/unit/schemas/test_promotion_monotonicity.py->8 passed in 4.97spython -m devtools test tests/unit/schemas/ tests/unit/core/test_runtime_registry_helpers.py->71 passed in 7.08spython -m devtools verify --quick->exit_code: 0(ruff format/check, mypy, render all, layering, closure-matrix, schema roundtrip, manifests, ci-workflows, doc-commands, docs-coverage, test-infra-currency, pytest-timeout-overrides, degrade-loudly, hash-boundary-census, schema-versioning policy, schema promotion audit)devtools verify --all(reserved for a broader pre-merge pass);tests/unit/core/test_sampling.py::TestLoadSamplesFromDb::test_schema_observation_records_included_and_decode_failed_rawsfails deterministically on this branch but is unrelated -- it does not importruntime_registryand the failure reproduces identically when run in isolation; classified as a pre-existing baseline failure, not caused by this change.Ref polylogue-ov5r
Summary by CodeRabbit
Bug Fixes
Tests