Skip to content

capabilities: add the pg-sprite capabilities command - #99

Merged
Kiran01bm merged 8 commits into
mainfrom
kiran01bm/cm3-capabilities-subcommand
Sep 10, 2026
Merged

capabilities: add the pg-sprite capabilities command#99
Kiran01bm merged 8 commits into
mainfrom
kiran01bm/cm3-capabilities-subcommand

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

Add an offline pg-sprite capabilities command for human and machine access to the binary's embedded support matrix.

Why

The generated support page is easy to read, but automation should not scrape Markdown and operators need to know which matrix their binary contains.

What

  • Add compact text output for area, operation, tier and mark, backend, front doors, and owner.
  • Add stable JSON output with the binary version and typed capability rows from pkg/capabilities.
  • Cover the contract queries in unit tests and add a packaged-binary demo check.
  • Document the output and mark contract step 2 complete.

Before / after

Before: support data was available only in docs/capabilities.md.

After:  pg-sprite capabilities --json | jq '.capabilities[] | select(.tier == "t2")'
        {
          "id": "add-column-with-volatile-default-now-gen-random-uuid",
          "tier": "t2",
          ...
        }

Refs: docs/capabilities-contract.md

… tables

docs/capabilities.md was hand-edited Markdown, so its tiers, marks, and
owner vocabulary could drift from the verdict reasons the engine emits
and from the contract in docs/capabilities-contract.md, and no machine
consumer could read it.

pkg/capabilities embeds capabilities.yaml (53 rows across 7 areas) and
exposes it as typed rows behind a small API. Loading validates the closed
vocabularies (tier, mark, backend, owner) and the cross-field rules
against the real verdict.Reasons(), so a row cannot name a reason the
engine does not have or carry an owner outside the tier that allows one.
A renderer emits the marked matrix tables and the summary counts between
markers in docs/capabilities.md; `make gen-capabilities` regenerates them
and a unit test fails when the checked-in document is stale or a marker
is malformed. Regeneration against the current document is a no-op and
the table cell text is unchanged.

This completes step 1 of the capabilities contract; the contract doc,
SAFETY.md periphery table, and go.mod (yaml.v3 promoted to a direct
dependency) are updated to match.
The support matrix is embedded in each binary, but users and automation could
only read its generated Markdown form. That left tools to scrape prose and
gave operators no quick terminal view of the binary's exact support claims.

Add an offline capabilities command with a compact text table and stable JSON
that pairs the typed rows with the release-stamped binary version. The command
uses the package row types directly, so its fields and enum values stay tied
to the validated YAML contract.

Cover the JSON query cases and text rows in unit tests, exercise the packaged
binary in the demo check, and document the new output and contract status.
Base automatically changed from kiran01bm/cm2-capabilities-source to main September 10, 2026 19:44
… check

The truncated `capabilities --json | head` example escaped the docs guard
because its command line does not end in `--json`, so the one block that
shows the new command could drift with every test green. Give it its own
section and TOC entry on the examples page, and add a guard that compares
the published lines byte for byte with what the command prints first.

The contract doc's example row promised `"diff": "not_applicable"` for
`refresh-materialized-view`; the registry says `refused` and nothing can
emit the other value. Correct the example and add a test that marshals the
named row and compares it with the printed object, so the promise stays
checkable.

The tour's version assertion accepted the literal string "null" that jq
prints for a missing key. Compare the JSON version with `--version` instead,
which is the claim the docs make.

🤖 Generated with Amp (Claude Opus 4.6)
…ities-subcommand

* origin/main:
  capabilities: make the embedded YAML matrix the source of the support tables (#98)
  verdict: classify every refusal with a typed class and owner (#97)
  fix(dbconn): strip explicit pg_catalog from pooled search_path (#93)

# Conflicts:
#	Makefile
#	docs/capabilities-contract.md
#	docs/capabilities.md
#	pkg/capabilities/capabilities.go
#	pkg/capabilities/capabilities.yaml
#	pkg/capabilities/capabilities_test.go
@Kiran01bm
Kiran01bm marked this pull request as ready for review September 10, 2026 20:07
@chatgpt-codex-connector

Copy link
Copy Markdown

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

@aparajon

Copy link
Copy Markdown
Collaborator

🤖 Reviewed 7d8fb6b8..e39f0fdc (9 files, +254/−14). CI green, 15/15. I ran a 14-mutant harness over internal/cli/capabilities.go, internal/cli/cli.go, and cmd/pg-sprite/main.go: 4 killed, 7 survived, 3 compile-skipped. The four killed are all on the JSON path, and every survivor is on the text path or the version wiring. Three findings.

Verified rather than taken from the description:

  • The JSON envelope is exactly what capabilities-contract.md pre-specified — one object with version and capabilities, source order preserved. Step 2's scope claim ("pg-sprite capabilities, including --json and the embedded binary version") is accurate.
  • All three "must support" recipes in the contract doc work against the real output. select(.tier == "t2") → 19, select(.front_doors.diff == "refused") → 35, select(.owning_tool_class != null) → 14 of 53. The third one is the fragile one — it needs the field absent, not empty — and Row.OwningToolClass carries omitempty, so it filters correctly instead of matching all 53.
  • TestCLIOutputExamplesCapabilitiesHeadMatchesCommand is the strongest test in the PR. It killed a renamed version key and an unindented encoder, neither of which any other test sees.

1. Nothing pins that the binary's real version reaches the JSON — three layers all default to "dev"

The feature's headline field is unprotected, and the mutation harness says so twice:

*** SURVIVED  m01 main forgets to pass the version (cli.New() is legal)
*** SURVIVED  m02 New ignores the version it was given

Every guard that looks like it covers this is defeated by the same default:

  • New(version ...string) (internal/cli/cli.go:37-42) makes the argument optional, so cli.New() compiles and silently yields "dev". A variadic converts what a plain parameter would enforce at compile time into a runtime default.
  • Both tests construct the command directly — CapabilitiesCmd{JSON: true, version: "test-version"} (capabilities_test.go:15) and {version: "dev"} (docs_test.go:155) — so New's wiring to Capabilities.version is never exercised at all.
  • capabilities_test.go:24 asserts Equal(cmd.binaryVersion(), response.Version), which compares the response against the same field the command read from. It pins the struct-to-JSON hop and cannot see the hop above it.
  • The demo assertion (demo/tour.sh:251) compares $PGS --version against jq -r .version, and its comment says it "catches a renamed or omitted key" — which it does. But make build (Makefile:23-25) passes no -ldflags, so bin/pg-sprite is "dev", and make demo-check runs that binary. Both sides read "dev" whether or not main.go passes the version, so the one check that spans the wiring is vacuous in the only build the repo produces.

The cheap fix is the type: New(version string) with the two call sites passing "dev". Then m01 is a compile error rather than a silent regression — the same guarantee kong.Vars{"version": version} already gives --version. If the variadic is there for source compatibility, note that internal/cli cannot be imported outside this module, so the only callers are main.go and this package's tests.

2. The header is misaligned against every data row

%-13s %-29s %-5s … (capabilities.go:35, :45-47) pads by runes, which is what Go's fmt width counts, but the terminal lays out by display cells — and every status mark is a double-width glyph. tierMark is t1/✅: 4 runes, padded to 5, rendered as 6 cells. So every row is one cell right of the header from BACKEND onward:

AREA          OPERATION                     TIER  BACKEND        FRONT DOORS    OWNER
column_chang… `ADD COLUMN` (no default, or… t1/✅  native_as_is   m:yes d:yes    —
column_chang… `ADD COLUMN` with volatile d… t2/🟡  copy_and_swap  m:ref d:ref    —

Measured by East Asian Width, the prefix up to the backend column is 50 cells in the header and 51 in every row — 50 runes either way, which is why it looks right in a rune-counting test and wrong in a terminal. ✅, 🟡, ⚪, 🔵, and ❌ are all wide, so all 53 rows shift equally and the header alone is out of step.

Simplest fix is to stop padding a column that contains a mark: drop %-5s for tier and print the mark in its own trailing column, or pad by measured display width. Both are cheaper than a width table.

Why no test could have caught it, which is the more useful half. The text path has a presence oracle and no content oracle — five separate content mutations survive:

*** SURVIVED  m05 clip is off by one at the boundary
*** SURVIVED  m07 a refused front door is labelled supported (ref -> yes)
*** SURVIVED  m08 the header row is never printed
*** SURVIVED  m09 the owner column shows blank instead of the em dash
*** SURVIVED  m10 the tier/mark cell loses the status mark
KILLED        m13 the first capability row is dropped

TestCapabilitiesTextListsEveryOperation pins exactly one property — every operation string appears somewhere in the output — and nothing about columns, labels, marks, or the header. cli-output-examples.md:5 declares human text "display only and unpinned," so that is policy rather than oversight for layout. It should not extend to m07: see 2/2.

3. clip slices the trimmed value using the untrimmed bound

func clip(value string, width int) string {
	if utf8.RuneCountInString(value) <= width {
		return value
	}
	return string([]rune(strings.TrimSpace(value))[:width-1]) + "…"
}

(capabilities.go:66-71)

The guard measures value; the slice indexes TrimSpace(value). When trimming removes more than len − (width−1) runes the bound outruns the trimmed length, and because []rune(string) allocates capacity above its length, this does not panic — it reads zeroed runes:

clip("            ab", 13)  ->  "ab\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00…"
clip("             a", 13)  ->  "a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00…"
clip("   abcdefghijkl   ", 13) -> "abcdefghijkl…"    // fine: trimming stays inside the bound

validateText (pkg/capabilities/capabilities.go:185-198) rejects only \r\n, so a quoted YAML scalar with leading spaces reaches the renderer. Nobody would write that value deliberately, so this is a latent path rather than a live bug — but it emits NUL bytes into operator output instead of failing, which is the wrong direction for a repo whose rule is that invariant violations return ErrInvariantViolation rather than degrading silently.

The TrimSpace is also the sole reason strings is imported here — mutant m06 (dropping it) compile-skipped on an unused import. Removing it makes the function total, since RuneCountInString(value) > width guarantees []rune(value) has at least width−1 elements.


Nothing here blocks. Finding 1 is the one worth taking before merge: it is a one-word signature change, and it is the difference between the version field being guaranteed and being conventional.

Tests, the docs sweep, and one comment placement in 2/2.


This review was generated by Claude Code (claude-opus-5).

@aparajon

Copy link
Copy Markdown
Collaborator

🤖 2/2 — tests, docs, and one comment placement. Correctness is in 1/2.

Credit first, because it is the best thing in the PR. TestContractDocExampleRowMatchesRegistry found real pre-existing drift on its first run: the contract doc's example row published "diff": "not_applicable" while capabilities.yaml's refresh-materialized-view row says diff: refused (pkg/capabilities/capabilities.yaml:466-468). I checked the YAML directly — the correction in this diff is the right direction, and the test is why it surfaced. Pinning the example a consumer copies into a jq filter against the registry, rather than eyeballing it, is exactly the shape that keeps a contract doc honest. The Status: line moving to "steps 1–2 are complete" is also accurate: step 2 is "add pg-sprite capabilities, including --json and the embedded binary version," which is this PR.


4. The new test was inserted between an existing comment and the function it documents

pkg/capabilities/capabilities_test.go:173-178 now reads:

// A pipe inside a cell is escaped so GFM keeps it in that cell; every
// other column stays in place and the escaped source renders as the pipe.
// The contract doc promises that one named row marshals to the JSON object
// it prints. Consumers copy that object's field values into jq filters, so
// the example must be the row's actual encoding, not a paraphrase of it.
func TestContractDocExampleRowMatchesRegistry(t *testing.T) {

The first two lines belong to TestRenderDocumentEscapesPipesInCells, which now sits at :199 with no comment at all. Two tests are mis-documented by one insertion point: the new test claims a rationale about GFM pipe escaping it has nothing to do with, and the pipe test loses the explanation of why it exists. Move the new comment and function below the pipe test, or move the two orphaned lines back down with it.

5. doorLabel is a closed-vocabulary translation, and it is unpinned

*** SURVIVED  m07 a refused front door is labelled supported (ref -> yes)

doorLabel (internal/cli/capabilities.go:54-63) maps three closed statuses onto three three-letter labels, and inverting one of them changes 35 rows from "the declarative door refuses this" to "the declarative door supports this" with no test failing.

This is worth separating from the layout question. cli-output-examples.md:5 declares human text "display only and unpinned," and that is a defensible policy for column widths, colors, and truncation — nobody should golden-test a table's whitespace. It reads differently for a function that translates a safety-relevant enum into the words an operator reads: "display only" covers how it looks, not whether it says the true thing. A three-case table test over DoorSupported, DoorRefused, and DoorNotApplicable costs four lines and makes the mapping total. (There is no exported closed-set accessor to walk — the validator inlines the three constants in its checkEnum call at pkg/capabilities/capabilities.go:168 — so a table is the available shape, and a fourth status added tomorrow would still fall through doorLabel's default rather than failing a test.) The same argument applies more weakly to m09/m10 — the em-dash owner placeholder and the status mark are content, not layout — but doorLabel is the one where an inversion is legible as a false statement rather than a cosmetic slip.

6. The text test uses the renderer's own clip as its oracle

assert.Contains(t, out.String(), clip(row.Operation, 29))

(internal/cli/capabilities_test.go:50)

Both sides of the comparison run the same function, so any change to clip moves the expectation with the output. This is why m05 (off-by-one at the boundary) survives, and a clip that returned the empty string would satisfy Contains(out, "") for all 53 rows. The 29 is also a second copy of the renderer's format-string width (capabilities.go:45), so the two drift independently.

A stronger oracle for the property this test wants — no row is silently dropped — does not need clip at all: assert the output has one line per row plus the header, and check a couple of known-short operations verbatim (`DROP COLUMN` fits in 29 and needs no clipping). That keeps m13 killed while making the assertion independent of the code under test. If the truncation itself is worth pinning, a direct table over clip is the place, not through the renderer.


Minor, since the test permits it. The doc block is labelled $ pg-sprite capabilities --json | head but publishes 9 lines; head prints 10 (the next line is "engine_path": "native_as_is",). TestCLIOutputExamplesCapabilitiesHeadMatchesCommand compares a prefix, so it passes either way — but the command line as written does not produce the block shown. Either publish the tenth line or drop | head from the prompt.

This review was generated by Claude Code (claude-opus-5).

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approving. Clean addition, and the strongest part is the one that wasn't the feature: TestContractDocExampleRowMatchesRegistry caught real pre-existing drift in the contract doc's example row, and all three of that doc's "must support" jq recipes check out against the real output. The JSON path is well pinned — a renamed version key and an unindented encoder both die on the captured-output test.

Six findings in the two comments above, none blocking. A 14-mutant harness came back 4 killed / 7 survived / 3 compile-skipped, and the split is informative: everything on the JSON path is covered, everything on the text path and the version wiring is not.

The one worth taking before merge is finding 1 — New(version ...string) makes cli.New() legal, so main.go dropping the version is a silent regression rather than a compile error, and each of the three checks that appear to cover it reads "dev" on both sides. Changing the signature to New(version string) turns that mutant into a build failure.

This stamp was left by Claude Code (claude-opus-5).

Kiran01bm and others added 3 commits September 11, 2026 07:26
…content

Address the review of the capabilities command:

- New takes the binary version as a plain parameter; a caller can no
  longer build the command tree and silently leave the capabilities
  output reporting a default. A test pins the version through New.
- The status mark is a double-width glyph, so it moves to an unpadded
  trailing STATUS column and the padded columns hold only single-width
  runes; the header now lines up with every row in a terminal.
- clip measures and slices the same string, so it can no longer read
  past the value and emit NUL bytes for a leading-space cell.
- The text test derives its expectations from the registry instead of
  the renderer's own clip: one line per row, a short operation verbatim,
  the mark in the same column on every line, the em dash for an empty
  owner. doorLabel and clip get direct table tests, including clip's
  boundary and leading-space cases.
- The pipe-escaping test gets its comment back; the contract-doc example
  test carries only its own.
- The published `capabilities --json | head` block shows the ten lines
  head prints.
- The contract doc marks completion per step, so parallel step PRs edit
  their own line instead of rewriting one shared status sentence.

🤖 Generated with Amp (Claude Opus 4.6)
…b.com/block/pg-sprite into kiran01bm/cm3-capabilities-subcommand

* 'kiran01bm/cm3-capabilities-subcommand' of https://github.com/block/pg-sprite:
  docs: decide the lock-budgeted passthrough contract (#102)
@Kiran01bm

Copy link
Copy Markdown
Collaborator Author

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

Verdict: all six findings and the minor note were accurate and are fixed; New now takes the version as a required parameter, the text table pads only single-width columns and prints the status mark as its own trailing column, clip is total, the two tests use oracles independent of the renderer, doorLabel is pinned, and the contract doc's status moved onto per-step markers so it merges cleanly with pull/100.

# Concern Status Explanation
C1-F1 New(version ...string) makes the version optional, so cli.New() compiles and yields "dev"; no test exercises the NewCapabilities.version hop, and the demo check compares "dev" to "dev". fixed New(version string) is non-variadic; main.go passes the ldflags-stamped version, every test call site passes "test". New TestNewWiresVersionIntoCapabilitiesJSON constructs the tree through New("v9.9.9-test"), runs capabilities --json, and asserts the decoded version is v9.9.9-test, so the wiring is covered by a value that is not the default. Mutant m01 (forgetting to pass) is now a compile error; m02 (ignoring the argument) fails the new test.
C1-F2 %-5s pads the t1/✅ cell by runes while the mark renders two cells wide, so every data row sits one cell right of the header from BACKEND onward. fixed The layout is one shared capabilitiesRowFormat (%-13s %-29s %-4s %-14s %-14s %-9s %s): TIER holds the bare tier token, and the double-width mark is the unpadded final STATUS column, so no padded column contains a wide glyph. Header and rows are printed from the same format string. Confirmed on the built binary: header and every row align.
C1-F3 clip measures value but slices TrimSpace(value), so a value with enough leading whitespace reads past the trimmed length and emits NUL runes. fixed The TrimSpace is gone; guard and slice measure the same string, so []rune(value)[:width-1] is always in range and the function is total. New TestClip table covers shorter-than, exactly-width, one-over, a multibyte value, and the leading-whitespace case (" ab", 13 → " …": the value is clipped as written, no NULs).
C2-F4 The new test was inserted between TestRenderDocumentEscapesPipesInCells's comment and its function, mis-documenting both. fixed The two pipe-escaping comment lines are back on TestRenderDocumentEscapesPipesInCells; TestContractDocExampleRowMatchesRegistry carries only its own comment.
C2-F5 doorLabel translates a closed safety-relevant enum into operator-facing words and no test pins it; inverting ref/yes changes 35 rows unnoticed. fixed TestDoorLabel is a table over DoorSupportedyes, DoorRefusedref, DoorNotApplicablen/a, plus the fall-through for an unknown status. Mutant m07 fails.
C2-F6 The text test used the renderer's own clip (and a second copy of the width 29) as its oracle, so clip regressions move the expectation with the output. fixed TestCapabilitiesTextLayout asserts line count = rows + 1, that `DROP COLUMN` appears verbatim, that the status mark sits at the same rune column on every data row, that an empty owner renders as the em dash, and that a long owner is clipped to a 5-rune prefix followed by — none of it via clip. Truncation itself is pinned directly by TestClip. Mutants m05, m08, m09, m10 all fail; m13 stays killed.
C2-minor The --json | head block shows 9 lines; head prints 10. fixed The block now shows the tenth line, "engine_path": "native_as_is",, so the command as written produces the block as shown.

The credit paragraph on TestContractDocExampleRowMatchesRegistry (real drift in the example row) needs no action.

Source: #99, review comments 5624989278 and 5624990165 and review 5171940472 at head e39f0fd

@Kiran01bm
Kiran01bm enabled auto-merge (squash) September 10, 2026 23:15
@Kiran01bm
Kiran01bm merged commit 321d5b0 into main Sep 10, 2026
15 checks passed
@Kiran01bm
Kiran01bm deleted the kiran01bm/cm3-capabilities-subcommand branch September 10, 2026 23:17
Kiran01bm added a commit that referenced this pull request Sep 10, 2026
…ities-ci-gate

* origin/main:
  fix(dbconn): keep the execution bounds where a pooler would drop them (#81)
  capabilities: add the pg-sprite capabilities command (#99)
  replay: assert the engine-emitted refusal class (#101)
  docs: decide the lock-budgeted passthrough contract (#102)

# Conflicts:
#	docs/capabilities-contract.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants