Summary
api-contract-reviewer's Consumer Test only evaluates what a hook consumer receives. It has no rule for what the caller does with the value the consumer returns. For WordPress filters, which are bidirectional by definition, that leaves an entire class of contract break invisible to the agent whose job is contract stability.
Where the gap is
From agents/api-contract-reviewer.md, the Consumer Test:
- Public interface: Is this consumed by code outside this changeset?
- Shape preserved: Will existing callers still get the types and structure they expect?
- Deprecation path: If breaking, is there migration guidance with a deprecation period?
If the answer to #1 is yes and #2 is no, it's a contract break.
Step 2 covers the inbound half only. A change that leaves apply_filters() untouched but alters how the returned value is subsequently handled passes the test cleanly, and the agent exits with a clearance exactly as instructed.
The category definitions reinforce the inbound framing. "Changed Hook Arguments" is defined as "Filter receives fewer arguments than before". The MEDIUM list has "Hook priority changed, altering execution order". There is a hook-contract-break category slug, but nothing in the file ever instantiates it as anything other than arity, argument types, or priority.
Minimal repro shape
$data = apply_filters( 'plugin_thing_data', $data );
foreach ( $data as $key => $value ) {
- $this->set( $key, normalize( $value ) );
+ $this->set( $key, $value );
}apply_filters() is byte-identical before and after. Arity, argument types, and priority are unchanged. But every third-party callback that returned a value relying on normalize() running afterwards now behaves differently, and if set() has a type guard, some of those values are silently dropped rather than applied.
The agent clears this. The Consumer Test has no question it fails.
The secondary problem: an invented bridge rule
This is arguably the more important half. Having no rule to apply, the agent filled the gap with reasoning that appears nowhere in its prompt. Paraphrasing an actual clearance it produced:
the docblocks never document a post-filter normalize step, so filter authors have no documented guarantee to depend on.
"Undocumented, therefore not a contract" is circular in a harmful direction. It treats absence of documentation as evidence of absence of contract, so the less documented a behavior is, the safer it appears to change. That inverts the real risk: long-standing undocumented behavior is where Hyrum's Law bites hardest, and it is precisely where integrators have no doc to re-read to discover the new rule.
It is also self-sealing against the correct finding. When the right outcome is "this behavior change should be documented," this shortcut uses the absence of that documentation as proof there is nothing to document.
Worth checking whether this reasoning pattern shows up in clearances from other agents too. A rule gap that gets filled by a plausible-sounding invented rule produces confident false clearances, which are harder to notice than a reviewer simply saying nothing.
Contributing factor: FALSE POSITIVE GATE item 2
- Is this internal refactoring? (Changed how a result is computed without changing the result)
A post-filter processing change pattern-matches to "changed how a result is computed," and the gate provides no instruction to verify the parenthetical actually holds. Where the result does change, the gate offers an unearned exit.
Suggested direction
I have not written a patch; flagging the shape in case it is useful.
- Add a return-side question to the Consumer Test, roughly: for a value-filtering hook, does the caller still do the same thing with what the consumer returns? This is the load-bearing change.
- Add a
hook-contract-break example that is neither arity nor priority, so the category has an instance covering post-filter handling.
- Explicitly rule out "undocumented, therefore not a contract" as grounds for a clearance. Established runtime behavior is a contract whether or not the docblock mentions it; if anything, undocumented behavior warrants more care because there is nowhere for consumers to learn the change.
- Tighten gate item 2 to require confirming the result is genuinely unchanged before discarding.
How it surfaced
A specialist clearance on a small diff, where several other reviewers in the same run independently raised the return-side change as a real finding. The review outcome was correct, so nothing was actually missed.
Filing it anyway for two reasons. The correct outcome depended on redundancy between agents, which will not always be present on a diff that dispatches fewer reviewers. And the fleet demonstrably already holds this knowledge in at least one place: woo-regression-reviewer lists "filter return-type variance" among its invariants. Wherever that phrasing came from, it belongs in the agent that owns contract stability generally, not only in one with a narrow dispatch scope.
Not urgent - a fleet-quality improvement rather than a defect postmortem.
Summary
api-contract-reviewer's Consumer Test only evaluates what a hook consumer receives. It has no rule for what the caller does with the value the consumer returns. For WordPress filters, which are bidirectional by definition, that leaves an entire class of contract break invisible to the agent whose job is contract stability.Where the gap is
From
agents/api-contract-reviewer.md, the Consumer Test:Step 2 covers the inbound half only. A change that leaves
apply_filters()untouched but alters how the returned value is subsequently handled passes the test cleanly, and the agent exits with a clearance exactly as instructed.The category definitions reinforce the inbound framing. "Changed Hook Arguments" is defined as "Filter receives fewer arguments than before". The MEDIUM list has "Hook priority changed, altering execution order". There is a
hook-contract-breakcategory slug, but nothing in the file ever instantiates it as anything other than arity, argument types, or priority.Minimal repro shape
apply_filters()is byte-identical before and after. Arity, argument types, and priority are unchanged. But every third-party callback that returned a value relying onnormalize()running afterwards now behaves differently, and ifset()has a type guard, some of those values are silently dropped rather than applied.The agent clears this. The Consumer Test has no question it fails.
The secondary problem: an invented bridge rule
This is arguably the more important half. Having no rule to apply, the agent filled the gap with reasoning that appears nowhere in its prompt. Paraphrasing an actual clearance it produced:
"Undocumented, therefore not a contract" is circular in a harmful direction. It treats absence of documentation as evidence of absence of contract, so the less documented a behavior is, the safer it appears to change. That inverts the real risk: long-standing undocumented behavior is where Hyrum's Law bites hardest, and it is precisely where integrators have no doc to re-read to discover the new rule.
It is also self-sealing against the correct finding. When the right outcome is "this behavior change should be documented," this shortcut uses the absence of that documentation as proof there is nothing to document.
Worth checking whether this reasoning pattern shows up in clearances from other agents too. A rule gap that gets filled by a plausible-sounding invented rule produces confident false clearances, which are harder to notice than a reviewer simply saying nothing.
Contributing factor: FALSE POSITIVE GATE item 2
A post-filter processing change pattern-matches to "changed how a result is computed," and the gate provides no instruction to verify the parenthetical actually holds. Where the result does change, the gate offers an unearned exit.
Suggested direction
I have not written a patch; flagging the shape in case it is useful.
hook-contract-breakexample that is neither arity nor priority, so the category has an instance covering post-filter handling.How it surfaced
A specialist clearance on a small diff, where several other reviewers in the same run independently raised the return-side change as a real finding. The review outcome was correct, so nothing was actually missed.
Filing it anyway for two reasons. The correct outcome depended on redundancy between agents, which will not always be present on a diff that dispatches fewer reviewers. And the fleet demonstrably already holds this knowledge in at least one place:
woo-regression-reviewerlists "filter return-type variance" among its invariants. Wherever that phrasing came from, it belongs in the agent that owns contract stability generally, not only in one with a narrow dispatch scope.Not urgent - a fleet-quality improvement rather than a defect postmortem.