[Kiran01bm]

pg-sprite diff now reports a typed advisory for every index left invalid by an unfinished concurrent build, so an empty plan no longer hides one.

Why

diff already plans a rebuild when the desired schema declares an index that is invalid on the live table, but it never says why the rebuild is there. When the desired schema does not name the invalid index (a typical leftover _ccnew from an interrupted REINDEX CONCURRENTLY, or an abandoned CREATE INDEX CONCURRENTLY), the diff never drops it, so the plan comes back empty and the table looks clean. docs/limitations.md and docs/pull.md documented this gap. An adapter or operator keying on "empty plan" had no machine-readable signal that the table still needs attention.

What

• pkg/schemadiff: a typed Advisory with two kinds.
  • invalid-index-rebuild: the desired schema declares the index (as is or redefined), and a rebuild is planned.
  • invalid-index-undeclared: the desired schema does not name the index, and nothing is planned.
• Plan report: an advisories array on diff-sourced plans, omitted when empty. FormatVersion goes from 14 to 15.
• Text output:
  • each advisory prints as a warning[<kind>] block linking the invalid-index runbook;
  • the summary line counts unfinished builds the plan leaves alone;
  • --sql adds a trailing comment line.
• Docs and demo:
  • capabilities.yaml and the generated capabilities.md, limitations.md, pull.md, plan-report.md (advisory kinds), cli-output-examples.md, schemabot-integration.md, invariants.md and the README are updated. limitations.md and pull.md now describe the gap as closed.
  • The demo tour seeds an abandoned unique build and asserts both advisory kinds.
• Not covered: a desired file that declares row security takes the row-security review path (PlanWithRowSecurity), which does not collect advisories. plan-report.md and schemabot-integration.md say so, so an absent field on that path is not read as "no invalid index".

Before / after

The same live table, with an index left invalid by an interrupted build, diffed against a desired file that does not declare that index:

live:     public.contacts, index contacts_email_key INVALID (unfinished build)
desired:  contacts without contacts_email_key

Before
  schemadiff   the index is undeclared, and diff never drops an index
  plan         "statements": []   (format_version 14, no other field)
  text         "no changes to run"
  operator     reads "clean" while an invalid index is still on the table

After
  schemadiff   the index is undeclared and invalid, so an advisory is raised
  plan         "statements": [],
               "advisories": [{"kind": "invalid-index-undeclared",
                               "index": "contacts_email_key"}]   (format_version 15)
  text         warning[invalid-index-undeclared]: index "contacts_email_key" is
               an unfinished concurrent build ... + runbook link
               "no changes to run; the live table carries 1 unfinished index
               build the plan leaves alone"

If the desired file does declare the index, the planned create-index rebuild now comes with a warning[invalid-index-rebuild] that explains it.

References

• Builds on schemadiff: plan a rebuild for an index left invalid by an unfinished build #86, which plans the rebuild for a declared invalid index.

🤖 Generated with Amp (Claude Opus 4.6)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-29T06:24:32.162321Z We couldn’t complete this review. Comment @codex review to try again.	2d1b840	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[JashLal]

🤖 Adversarial correctness review requested by Jash and performed by his AI agent, at head 2d1b840db1ecf75d24ad489767f32c674e66f6bd.

Verdict: no concrete executable-code blocker found in the full diff; one documentation correction below. The new observations remain separate from executable statements and the fingerprint, and distinguish a desired-name rebuild from an undeclared leftover without introducing a drop.

Findings

1. (non-blocking) The “Remove or redefine” limitation row conflates the advisory kinds. In docs/limitations.md:162, the new sentence names invalid-index-undeclared for a row covering both removal and redefinition. A same-name redefinition instead selects invalid-index-rebuild in pkg/schemadiff/advisory.go:74–76, as the new redefinition test also expects. Readers should not expect the undeclared kind when a create is planned.

Action items

1. (Finding 1) Qualify invalid-index-undeclared as the removal/absent-name case and name invalid-index-rebuild for redefinition under the same name.

Verified and limitations

Static full-diff review covered both kinds, multiple invalid entries, partitioned-parent exclusion, JSON omission/shape, format version 15, fingerprint exclusion, and warning/SQL-comment rendering. Added tests cover real abandoned builds and an in-flight build observation. No test functions were deleted; the demo replaces the zero-diff expectation only for its new invalid-index fixture and keeps it for the other tables. GitHub reports 16 successful checks, including PostgreSQL 14–18. I did not run a local build or tests. The exact-head source API request failed, so unchanged helper implementations, full call paths, and inline review threads were not independently verified; CI is corroboration, not local validation.

Generated by Codex, with an independent static review by GPT-6.1 Sol.

[JashLal]

🤖 Product review at 2d1b840, from Jash’s AI agent.

OSS adoption: Naming the index, exposing the typed warning, and linking the recovery runbook make an empty plan more useful. Small description correction: the PR body and example still say format version 13, while the actual implementation and public contract now use 15; update those and remove the obsolete merge-order/version note.

SchemaBot integration: The typed advisories field gives an adapter a signal without parsing prose or changing executable-plan identity. Please carry the acknowledged PlanWithRowSecurity exception into docs/plan-report.md:70 and the integration guidance: currently advisories are populated by diffplan.Plan, while the row-security path does not collect them. An absent field on that path therefore does not establish that the table has no unfinished indexes. This is a non-blocking clarification of the explicitly deferred scope, not a newly verified runtime regression.

Generated by Codex. Static review only; no live adapter validation or local tests.

[JashLal]

🤖 Approving on Jash’s behalf after the full-diff reviews at 2d1b840. No concrete executable-code blocker found. The advisory-kind, row-security coverage, and stale version-description corrections are recorded above for the author. Exact-head surrounding-source and inline-thread verification were unavailable; no local build or tests ran. Generated by Codex.

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — pull/249, follow-up commit

All three findings are fixed: two documentation corrections in the follow-up commit, and the description refreshed to format version 15.

#	Finding	Status	Explanation
C1-F1	The "Remove or redefine" limitation row names only invalid-index-undeclared, but a same-name redefinition raises invalid-index-rebuild	fixed	The row now names both: invalid-index-undeclared when the desired file no longer names the index, invalid-index-rebuild when it redefines it under the same name, matching Advisories in pkg/schemadiff/advisory.go.
C2-F2	The row-security path collects no advisories, so an absent field there proves nothing	fixed	plan-report.md's advisories row and the dry-run guidance in schemabot-integration.md now say a desired file that declares row security collects no advisories, so absence on that path does not mean the table has no invalid index.
C2-F1	The PR description still says format version 13 and carries an obsolete merge-order note	fixed	The description now says 14 → 15 throughout and drops the merge-order note.

Source: block/pg-sprite#249, review comments 6490404255 and 6490408052 and review 5694075394 at head 2d1b840d; fixes in the follow-up commit.