morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #7135] [MERGED] revert(mcp): undo input schema fitting that broke main CI #7664

New issue

Closed

opened 2026-11-03 13:39:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:29 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7135
Author: @kaoru0822-kitauji
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/26/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: revert/7119-input-schema-fitting

📝 Commits (1)

fac354d revert(mcp): undo input schema fitting that broke main CI

📊 Changes

13 files changed (+81 additions, -1199 deletions)

View changed files

📝 docs/mcp-contracts.md (+4 -22)
📝 scripts/verify-copilot-client.mjs (+2 -2)
📝 src/catalogIdentity.ts (+5 -3)
➖ src/contracts/inputSchemaFitInvariants.test.ts (+0 -264)
➖ src/contracts/inputSchemaPresentation.test.ts (+0 -371)
📝 src/contracts/inputSchemaPresentation.ts (+46 -423)
📝 src/contracts/toolSchemaMetadata.ts (+6 -9)
📝 src/server/EvidenceMcpServerCore.ts (+5 -3)
📝 tests/boundary/mcp/browserComparisonInput.test.ts (+2 -6)
📝 tests/boundary/mcp/browserScenarioMcp.test.ts (+6 -11)
📝 tests/boundary/mcp/compactInputSchemaMcp.test.ts (+2 -46)
📝 tests/boundary/mcp/toolSchemaValidity.test.ts (+2 -32)
📝 tests/fixtures/mcpClientSchemaConstraints.ts (+1 -7)

📄 Description

What broke: main CI fails at 25f0f9934 (run 40193564884) in 10 schema-advertisement tests, e.g. toolInputReferences "preserves the complete inline Evidence contract", toolSchemaValidity "advertised schema rejects modules/index", analysisViewMcp, absoluteLocalPathSchema, javascriptFactsViewMcp, godotPckMcp.
Fix: revert #7119 (64 KiB input-schema fitting). Its CI ran against bc143125e; once #7120 and the other merges grew inspect_analysis_view, fitting fell from instance level 3 to level 2 and summarized the inline Evidence source and view selectors.
Risk: low; restores the exact pre-#7119 advertisement. The full (opt-in) profile again exceeds Claude Desktop's 64 KiB per-tool limit for compare_web_captures and inspect_analysis_view (#4081), as it did before; the default compact profile is unaffected.

Summary

Refs #4081 (reopened by this revert; a follow-up will redesign the limit against these contracts).

Problem and expected behavior

On main 25f0f9934 the complete inspect_analysis_view input projection is 71,762 bytes plus 3,003 bytes of examples. Measured candidates from #7119's fitter: level 3 = 64,605 bytes before examples and guidance, so it no longer fits; level 2 = 50,924 bytes, which drops the inline Evidence required list and the view selector collection constraints that the failing tests assert. On #7119's branch base (bc143125e) the schema was smaller and level 3 fit, so its CI passed.

I also tried keeping small subschemas at the last level (summarizing only large ones). It still summarized the 7 KB inline Evidence object, so it doesn't satisfy these contracts; restoring the full advertisement needs a representation change, not a different cut. That is out of scope for a main-health fix.

Change and scope

git revert 7336a566a; applied cleanly on top of #7107/#7127. Contribution scope: not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: advertised input-schema presentation (src/contracts/inputSchemaPresentation.ts, toolSchemaMetadata.ts), back to its pre-#7119 state.
CLI and MCP/tool-catalog contract: full-profile advertisement returns to the complete projection; numeric const/enum relaxation from #7119 is also reverted. Compact profile unchanged.
Generated metadata, package, or installation impact: full-profile catalog digest returns to its pre-#7119 value.
Documentation or usability only; no target interaction.

Evidence and regression coverage

Base reproduction: on origin/main 25f0f9934, npx vitest run src/contracts/toolInputReferences.test.ts fails 2/34 with the same assertion as CI.
After the revert: npm run test:focused -- on the 10 files that failed or are touched (toolInputReferences, toolSchemaValidity, analysisViewMcp, absoluteLocalPathSchema, javascriptFactsViewMcp, godotPckMcp, workflowInputIdentities, compactInputSchemaMcp, browserComparisonInput, browserScenarioMcp): 10 files, 101 tests passed.
workflowInputIdentities timed out in the failing CI run; it passes locally here, so I can't tell whether it was related.

Validation performed

npm run test:focused -- <10 files above> — 101 passed
npm run check:fast — passed
npm run format:check — passed
npm run docs:check — passed

Compatibility, safety, and release

Breaking changes or migration steps: none beyond undoing #7119, which hasn't been released.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Security, privacy, process, or containment review: none.

Author readiness

Human owner: @kaoru0822-kitauji

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7135 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/26/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `revert/7119-input-schema-fitting` --- ### 📝 Commits (1) - [`fac354d`](https://github.com/morluto/rea/commit/fac354de4447f1885e7af57cb90bd17f205230cd) revert(mcp): undo input schema fitting that broke main CI ### 📊 Changes **13 files changed** (+81 additions, -1199 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+4 -22) 📝 `scripts/verify-copilot-client.mjs` (+2 -2) 📝 `src/catalogIdentity.ts` (+5 -3) ➖ `src/contracts/inputSchemaFitInvariants.test.ts` (+0 -264) ➖ `src/contracts/inputSchemaPresentation.test.ts` (+0 -371) 📝 `src/contracts/inputSchemaPresentation.ts` (+46 -423) 📝 `src/contracts/toolSchemaMetadata.ts` (+6 -9) 📝 `src/server/EvidenceMcpServerCore.ts` (+5 -3) 📝 `tests/boundary/mcp/browserComparisonInput.test.ts` (+2 -6) 📝 `tests/boundary/mcp/browserScenarioMcp.test.ts` (+6 -11) 📝 `tests/boundary/mcp/compactInputSchemaMcp.test.ts` (+2 -46) 📝 `tests/boundary/mcp/toolSchemaValidity.test.ts` (+2 -32) 📝 `tests/fixtures/mcpClientSchemaConstraints.ts` (+1 -7) </details> ### 📄 Description **What broke:** main CI fails at 25f0f9934 (run 40193564884) in 10 schema-advertisement tests, e.g. `toolInputReferences` "preserves the complete inline Evidence contract", `toolSchemaValidity` "advertised schema rejects modules/index", `analysisViewMcp`, `absoluteLocalPathSchema`, `javascriptFactsViewMcp`, `godotPckMcp`. **Fix:** revert #7119 (64 KiB input-schema fitting). Its CI ran against bc143125e; once #7120 and the other merges grew `inspect_analysis_view`, fitting fell from instance level 3 to level 2 and summarized the inline Evidence source and view selectors. **Risk:** low; restores the exact pre-#7119 advertisement. The full (opt-in) profile again exceeds Claude Desktop's 64 KiB per-tool limit for `compare_web_captures` and `inspect_analysis_view` (#4081), as it did before; the default compact profile is unaffected. ## Summary Refs #4081 (reopened by this revert; a follow-up will redesign the limit against these contracts). ## Problem and expected behavior On main 25f0f9934 the complete `inspect_analysis_view` input projection is 71,762 bytes plus 3,003 bytes of examples. Measured candidates from #7119's fitter: level 3 = 64,605 bytes before examples and guidance, so it no longer fits; level 2 = 50,924 bytes, which drops the inline Evidence `required` list and the view selector collection constraints that the failing tests assert. On #7119's branch base (bc143125e) the schema was smaller and level 3 fit, so its CI passed. I also tried keeping small subschemas at the last level (summarizing only large ones). It still summarized the 7 KB inline Evidence object, so it doesn't satisfy these contracts; restoring the full advertisement needs a representation change, not a different cut. That is out of scope for a main-health fix. ## Change and scope `git revert 7336a566a`; applied cleanly on top of #7107/#7127. Contribution scope: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: advertised input-schema presentation (`src/contracts/inputSchemaPresentation.ts`, `toolSchemaMetadata.ts`), back to its pre-#7119 state. - CLI and MCP/tool-catalog contract: full-profile advertisement returns to the complete projection; numeric `const`/`enum` relaxation from #7119 is also reverted. Compact profile unchanged. - Generated metadata, package, or installation impact: full-profile catalog digest returns to its pre-#7119 value. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Base reproduction: on origin/main 25f0f9934, `npx vitest run src/contracts/toolInputReferences.test.ts` fails 2/34 with the same assertion as CI. - After the revert: `npm run test:focused --` on the 10 files that failed or are touched (toolInputReferences, toolSchemaValidity, analysisViewMcp, absoluteLocalPathSchema, javascriptFactsViewMcp, godotPckMcp, workflowInputIdentities, compactInputSchemaMcp, browserComparisonInput, browserScenarioMcp): 10 files, 101 tests passed. - `workflowInputIdentities` timed out in the failing CI run; it passes locally here, so I can't tell whether it was related. ## Validation performed - `npm run test:focused -- <10 files above>` — 101 passed - `npm run check:fast` — passed - `npm run format:check` — passed - `npm run docs:check` — passed ## Compatibility, safety, and release - Breaking changes or migration steps: none beyond undoing #7119, which hasn't been released. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:39:29 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7664

No description provided.

Rows
Columns