Skip to content

perf(mcp): outputSchema was 80% of the tool payload and 54% of the context - #2529

Open
rubenvdlinde wants to merge 3 commits into
developmentfrom
fix/mcp-output-schema-payload
Open

perf(mcp): outputSchema was 80% of the tool payload and 54% of the context#2529
rubenvdlinde wants to merge 3 commits into
developmentfrom
fix/mcp-output-schema-payload

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

The finding

tools/list against /apps/openregister/api/mcp, measured 2026-08-16:

122 tools · 433,198 bytes · ~108,299 tokens
= 54% of a 200K context window, re-sent every turn

One field was four fifths of it:

Field Bytes Share
outputSchema 335,580 79.7%
inputSchema 59,022 14.0%
description 19,472 4.6%

The cause — an asymmetry nobody could see

buildOutputSchema() inlined $schema->getProperties() in full, for both read verbs.

Its input counterpart does the opposite: buildInputSchema() narrows a search verb to its declared filters (REQ-DERIVED-004). The input path was designed to be economical; the output path wasn't, and nothing surfaced the difference.

Worst case — shillinq.ARInvoice.search:

36,293 B  outputSchema
 1,915 B  inputSchema     ← 94% of the tool is the output schema

That one tool costs ~9,600 tokens by itself — more than hermiq's entire 22-tool set.

The change

Emit the envelope, not the item. search keeps {results, total, hasMore} with results.items as a bare object; get becomes a bare object.

The item's properties are redundant — the model reads the actual result when the tool returns. The envelope is not: it stops the model guessing whether search yields a wrapper or a bare array.

Kept the envelope rather than dropping outputSchema outright. Measured, that would save a further 6,688 B (1.5%) and lose the shape information.

Measured live, after deploying

Before After
payload 411,561 B 97,479 B −76%
context use 54% 12%
initialize 1221–1651 ms 992–1184 ms −20%
tools/list 1199–1417 ms 958–988 ms −25%

⚠️ The latency did NOT fall proportionally

A 76% smaller payload bought only ~20–25% off the handshake — so the ~2 s that remains is not serialisation-bound. It's schema enumeration and descriptor construction.

This change does not bring the handshake within the 250 ms budget (still ~8× over). Citing it as a latency fix would overstate it. That's the caching lever, deliberately a separate change with its own invalidation question.

Two details worth a reviewer's eye

The unused Schema parameter is removed — not tidiness. Leaving it would tell the next reader this method legitimately depends on the schema's properties, which is exactly the assumption that would re-inline them.

A byte-budget test makes a regression fail loudly. A payload regression is otherwise invisible: no test fails, no gate fires, nothing errors — it surfaces as agents getting slower and dumber, which gets blamed on the model. The test targets search+get only and says why in-line: create/update legitimately inline the property set into their input schema (~7.5 kB each) and would swamp the signal.

Risk

A client validating structured content per-property loses that. Nothing in this fleet does — hermiq's runner passes tool definitions through unchanged. Called out rather than buried.

16,475 tests, 0 failures. phpcs clean.

Spec: openspec/changes/mcp-output-schema-payload/

Conduction Release Bot
perf(mcp): outputSchema was 80% of the tool payload and 54% of the co…
ea23da8 
…ntext

Measured on the development instance, tools/list against
/apps/openregister/api/mcp:

  122 tools, 433,198 bytes, ~108,299 tokens
  = 54% OF A 200K CONTEXT WINDOW, re-sent every turn

One field was four fifths of it:

  outputSchema   335,580 B   79.7%
  inputSchema     59,022 B   14.0%
  description     19,472 B    4.6%

buildOutputSchema() inlined $schema->getProperties() IN FULL for both read
verbs. Its input counterpart does the opposite -- buildInputSchema()
narrows a search verb to its DECLARED FILTERS (REQ-DERIVED-004). The
input path was designed to be economical, the output path was not, and
nothing made the difference visible. Worst case shillinq.ARInvoice.search:
36,293 B of outputSchema against 1,915 B of inputSchema -- 94% of the
tool, and more tokens by itself than hermiq's entire 22-tool set.

Now emits the ENVELOPE, not the item: search keeps {results, total,
hasMore} with results.items as a bare object; get is a bare object. The
item's properties are redundant because the model reads the actual result
when the tool returns; the envelope is not, because it stops the model
guessing whether search yields a wrapper or a bare array.

Kept the envelope rather than dropping outputSchema altogether: measured,
that would save a further 6,688 B (1.5%) and lose the shape information.

MEASURED ON THE LIVE INSTANCE AFTER DEPLOYING:

  payload      411,561 B -> 97,479 B    -76%   (~108,300 -> ~24,400 tok)
  context use       54%  ->     12%
  initialize   1221-1651 -> 992-1184 ms  -20%
  tools/list   1199-1417 -> 958-988 ms   -25%

THE LATENCY DID NOT FALL PROPORTIONALLY, and the proposal says so. A 76%
smaller payload bought ~20-25% off the handshake, so the ~2 s that remains
is NOT serialisation-bound -- it is schema enumeration and descriptor
construction. This change does what it is for (context budget, and the
size of every prompt the model reads) and does NOT bring the handshake
within the 250 ms budget. Citing it as a latency fix would overstate it;
that is the caching lever, deliberately a separate change.

Also removed the now-unused Schema parameter. Not tidiness: leaving it
would tell the next reader this method legitimately depends on the
schema's properties, which is the assumption that would re-inline them.

Added a byte-budget test so a regression fails loudly. A payload
regression is otherwise invisible -- no test fails, no gate fires, nothing
errors; it surfaces as agents getting slower and dumber, which gets
blamed on the model. The test targets search+get only and says why:
create/update legitimately inline the property set into their INPUT
schema (~7.5 kB each on the fixture) and would swamp the signal.

16475 tests, 0 failures. phpcs clean.
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 2fe95be

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ❌
psalm ✅
phpstan ❌
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-specs ✅
test-l10n ✅
format ✅
composer ✅ ✅ 175/175
npm ✅ ✅ 528/528
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ✅
Newman ❌
Playwright ✅
Hydra gates ❌

Quality workflow — 2026-08-16 15:04 UTC

Download the full PDF report from the workflow artifacts.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ bda23bd

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-specs ✅
test-l10n ✅
format ✅
composer ✅ ✅ 175/175
npm ✅ ✅ 528/528
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ❌
Newman ✅
Playwright ✅
Hydra gates ❌

Quality workflow — 2026-08-17 00:58 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

rubenvdlinde commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

[CREW-B] Not merged — and the red cell is the ratchet inverting, not a broken test

Base is development ✓. I merged origin/development in first (ea23da87 → d311ed2a), because the original parity read charged this PR with four failures it did not cause. That part is now settled; one real blocker remains.

The four "introduced" failures were the base being repaired, not this PR

Branch point was 67686f3c (16:28 CEST). Every charged row maps to a fix that landed on development after it, and the file lists do not intersect this PR at all:

charged row repaired by files
PHP Quality (phpmd), PHP Quality (phpstan) #2533 SchemaNotInRegisterException.php, RegisterSchemaLinkageRepairService.php
Newman API Test Suite, Integration Tests (Newman) #2535 "clears all 3 jobs #2526 turned red" SchemasController.php, ObjectService.php, openregister-crud.postman_collection.json, +7

This PR touches lib/Mcp/BuiltIn/SchemaDerivedToolProvider.php, its unit test, and openspec/changes/mcp-output-schema-payload/**. Overlap: zero. 4 of 4 accounted for; all four are green after the merge-in. A rerun would have "fixed" this and been logged as flake.

What actually remains: PHPUnit (PHP 8.3, NC stable34, pgsql)

Authoritative run 31982803658 (36 jobs, total_count == jobs, untruncated). Base run 31980400031 on 1749c1d3 is green on all six PHPUnit cells, so this row is genuinely introduced — parity is not satisfied and I am not admin-merging past it.

But it is not a test failure. The suite passes: Tests: 16657, Assertions: 37520, zero failures, zero errors. The job fails at the coverage guard:

Changed PHP files in this PR: 2
Scoped to 2 changed PHP file(s).
Changed files, head:    95.91%  (305/318 statements)
Changed files, base:    95.96%  (309/322 statements)
FAIL: coverage of the files this change touches dropped by 0.05%.
      base 309/322 -> head 305/318 statements.

Read the counts, not the percentage — this is the deletion case

statements  322 -> 318   (-4)
covered     309 -> 305   (-4)
UNCOVERED    13 ->  13   ( 0)

Every one of the 4 removed statements was a covered statement, and the change added no uncovered statement whatsoever. Nothing became less tested. The ratio falls only because deleting covered statements from a 96%-covered file drags the mean down — 305×322 = 98 210 < 309×318 = 98 262, so cgRatioDropped() is true by exactly that margin.

This is the known inversion: the ratchet is right for ADDITIONS and backwards for DELETIONS, and .github#480 (S36/coverage-guard-deletion-neutral) is the asymmetric fix — drop base-only statements, keep head-only. It is DRAFT, pending DECISION-1 (Ruben's call).

🔑 Worth stating plainly, because two PRs are now red for what looks like the same reason and they are opposites:

  • docudesk#633 — +79 statements without tests. The ratchet is RIGHT; it waits for coverage.
  • openregister#2529 — -4 statements, all of them covered, 13 uncovered untouched. The ratchet is WRONG; it waits for .github#480.

Recommendation: hold until .github#480 lands, then re-run this cell — it should pass untouched. Please do not clear it by adding tests for the 13 pre-existing uncovered statements; that buys a ratchet back with filler and hides the defect that .github#480 exists to fix.

(The change itself reads well and is measured — outputSchema 79.7% → envelope-only, 335 580 B of a 433 198 B tools/list payload recovered, with the "keep the envelope, drop the item" trade-off costing a stated 1.5%. The single behavioural risk — a client validating structured content per-property — is called out in design.md rather than buried. No objection to the content.)

Conduction Release Bot
fix(validation): an enum error must say what the allowed values ARE
31b8b3b 
The message rendered as:

  Property 'ticketType' should be one of: , but is 'contactmoment'.
  Please choose one of the allowed values.

An EMPTY allowed-list — the one fact the reader needs is the one it omits.

Cause: Opis's EnumKeyword raises `$this->error($schema, $context, 'enum', 'The
data should match one item from enum')` with NO fourth argument, so `$args` is
empty and `$args['values']` could never be populated. The formatter was reading a
key nothing ever sets. The values are taken from the schema instead.

WHY IT MATTERS MORE THAN A COSMETIC MESSAGE. Measured 2026-08-17: an agent told
"should be one of: , but is 'sent'" concluded the enum was empty and that NO value
could be valid, and worked around a constraint that was correctly rejecting its
input. I read it the same way and spent time hunting a non-existent schema bug.
Both enums were fine — ticketType wants one of request|complaint|interaction, and
outcome one of handled|transferred|callbackRequest|followUpAction|resolved|
referred. The schema was right; the message was wrong.

A self-correcting caller — an LLM deciding what to send next — needs the list.
So does a human reading a 400.

After:

  Property 'ticketType' should be one of: 'request', 'complaint', 'interaction',
  but is 'contactmoment'. Please choose one of the allowed values.

Verified against the live instance with the same rejected payload.
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 0b65c24

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-specs ✅
test-l10n ✅
format ✅
composer ✅ ✅ 175/175
npm ✅ ✅ 528/528
app:check-code ⏭️
info.xml ✅
REUSE ❌
PHPUnit ✅
Newman ✅
Playwright ✅
Hydra gates ❌

Quality workflow — 2026-08-17 06:08 UTC

Download the full PDF report from the workflow artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rubenvdlinde