morluto/rea

Fork 0

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

[PR #3920] [CLOSED] fix(contracts): include ref siblings in presentation budgets #5965

New issue

Closed

opened 2026-10-31 01:10:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3920
Author: @Afloat16
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/schema-reference-siblings

📝 Commits (1)

6ae18f9 fix(contracts): include ref siblings in presentation budgets

📊 Changes

3 files changed (+168 additions, -15 deletions)

View changed files

📝 src/contracts/inputSchemaPresentation.ts (+2 -1)
📝 src/contracts/toolInputReferences.test.ts (+45 -0)
📝 tests/boundary/mcp/toolSchemaValidity.test.ts (+121 -14)

📄 Description

Summary

The presentation counter skips schema-valued $ref siblings. Under a conservative schema-position policy, 16 properties with both a target reference and an allOf reference plus one extra reference contain 33 reference occurrences; the base counts 17 and keeps the shared form at budget 32. This change makes that policy count every supported schema-valued position. The actual Pi versions tested below use only 17 expansions for this sibling fixture, so this is not a reproduced Pi field-loss case.

Continue through schema-valued siblings after following the target. Keep the caller's reference path for siblings, preserve cycle termination, and leave literal default/const/enum/example data opaque. Keep SDK-level selection checks aligned with that policy.

Refs https://github.com/morluto/rea/discussions/3847, candidate 1. This is a separate focused fix.

Contract and boundary impact

Owner: input-schema presentation selection, followed by SDK advertisement.
Existing canonical schemas, operation names, validation authority and default budget remain unchanged. This corrects when the existing inline fallback is selected; it adds no target execution, network or persistence capability.
The SDK regression checks advertised selection, schema-position counts and canonical validation. Its local counter is a policy check; actual client-renderer behavior is verified separately below.
Ordinary fix within existing boundaries; no new provider or advance scope decision applies. No package/version or tracked generated-file change.

Evidence and validation

Pinned Node 24.18.0 / npm 11.16.0 and locked dependencies.

Source suite: 22 pass. Against the base, both added regressions fail (1 vs 3 sibling expansions; 4 vs 6 with recursive paths) while 20 existing tests pass.
Actual in-memory MCP SDK boundary suite: 15 pass, including listTools after conversion, 32 shared vs 33 inline, Ajv valid/invalid inputs and valid SDK tool calls. The new SDK case fails against the base because the 33-expansion form remains shared.
Official npm run compile:termux, skill/MCP/product catalog generation and portable ledger generation pass. npm run docs:check:built passes; README/onboarding consistency tests 5 pass. The portable ledger explicitly retains two unsupported provider lanes; this does not claim installed-provider coverage.
node --max-old-space-size=3072 node_modules/typescript6/lib/tsc.js --noEmit, npm run lint, npm run knip, npm run metadata:check, npm run verify:architecture-guards, changed-file formatting, pre-commit checks and git diff --check pass.

Published Pi renderer probe

Executed the shipped schemaToType, renderToolSignature and renderDeclarations from integrity-verified @earendil-works/pi-codemode 1.1.0 and 1.2.0 against eight schemas returned by a real MCP SDK 2.3.1 listTools exchange: 16 renderer executions on Node 24.18.0 / Zod 4.4.3, without adding a repository dependency.

Both sibling fixtures (32/33 conservative reference occurrences) render every input field without unknown; these Pi versions expand only 16/17 target references and ignore reference siblings.
An independent direct-reference control reproduces the actual cutoff: 32 references stay typed; the 33rd renders zz_extra: unknown. REA's existing inline fallback restores zz_extra: string in both versions.
Pi 1.2.0 renders the sibling fixture's input in 296 characters shared versus 636 inline. The conservative selector can therefore inline earlier than that renderer needs; no field-loss repair or efficiency gain is claimed for the sibling fixture.

Pinned renderer sources: 1.1.0 and 1.2.0. This isolates published renderer behavior after SDK conversion; an interactive Pi host/model session remains unverified.

Limits: the standard npm run check stops in the pinned native TypeScript 7 compiler because this host lacks /proc/self/exe; a passing native gate is not claimed. A whole-repository format attempt in this environment also fails in untouched HTML workers at uv_resident_set_memory; the three changed TypeScript files pass. SDK loopback and isolated renderer checks do not establish an interactive Pi/client session, a full CLI/provider workflow, performance gains or full CI coverage.

Current-main status

Superseded by 5e30969, which inlines advertised inputs at their projection owner and removes the reference-budget mechanism and tests. This patch no longer applies to current main; resolving its conflicts by restoring the deleted counter would revive a superseded representation. The validation above describes the historical branch, not current-main acceptance. Details: https://github.com/morluto/rea/pull/3920#issuecomment-6201011007

_{🔄 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/3920 **Author:** [@Afloat16](https://github.com/Afloat16) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/schema-reference-siblings` --- ### 📝 Commits (1) - [`6ae18f9`](https://github.com/morluto/rea/commit/6ae18f9b389be7e4aca5b937dc6ec6be081d2856) fix(contracts): include ref siblings in presentation budgets ### 📊 Changes **3 files changed** (+168 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `src/contracts/inputSchemaPresentation.ts` (+2 -1) 📝 `src/contracts/toolInputReferences.test.ts` (+45 -0) 📝 `tests/boundary/mcp/toolSchemaValidity.test.ts` (+121 -14) </details> ### 📄 Description ## Summary The presentation counter skips schema-valued `$ref` siblings. Under a conservative schema-position policy, 16 properties with both a target reference and an `allOf` reference plus one extra reference contain 33 reference occurrences; the base counts 17 and keeps the shared form at budget 32. This change makes that policy count every supported schema-valued position. The actual Pi versions tested below use only 17 expansions for this sibling fixture, so this is not a reproduced Pi field-loss case. Continue through schema-valued siblings after following the target. Keep the caller's reference path for siblings, preserve cycle termination, and leave literal default/const/enum/example data opaque. Keep SDK-level selection checks aligned with that policy. Refs https://github.com/morluto/rea/discussions/3847, candidate 1. This is a separate focused fix. ## Contract and boundary impact - Owner: input-schema presentation selection, followed by SDK advertisement. - Existing canonical schemas, operation names, validation authority and default budget remain unchanged. This corrects when the existing inline fallback is selected; it adds no target execution, network or persistence capability. - The SDK regression checks advertised selection, schema-position counts and canonical validation. Its local counter is a policy check; actual client-renderer behavior is verified separately below. - Ordinary fix within existing boundaries; no new provider or advance scope decision applies. No package/version or tracked generated-file change. ## Evidence and validation Pinned Node 24.18.0 / npm 11.16.0 and locked dependencies. - Source suite: **22 pass**. Against the base, both added regressions fail (1 vs 3 sibling expansions; 4 vs 6 with recursive paths) while 20 existing tests pass. - Actual in-memory MCP SDK boundary suite: **15 pass**, including listTools after conversion, 32 shared vs 33 inline, Ajv valid/invalid inputs and valid SDK tool calls. The new SDK case fails against the base because the 33-expansion form remains shared. - Official `npm run compile:termux`, skill/MCP/product catalog generation and portable ledger generation pass. `npm run docs:check:built` passes; README/onboarding consistency tests **5 pass**. The portable ledger explicitly retains two unsupported provider lanes; this does not claim installed-provider coverage. - `node --max-old-space-size=3072 node_modules/typescript6/lib/tsc.js --noEmit`, `npm run lint`, `npm run knip`, `npm run metadata:check`, `npm run verify:architecture-guards`, changed-file formatting, pre-commit checks and `git diff --check` pass. ### Published Pi renderer probe Executed the shipped `schemaToType`, `renderToolSignature` and `renderDeclarations` from integrity-verified `@earendil-works/pi-codemode` 1.1.0 and 1.2.0 against eight schemas returned by a real MCP SDK 2.3.1 `listTools` exchange: 16 renderer executions on Node 24.18.0 / Zod 4.4.3, without adding a repository dependency. - Both sibling fixtures (32/33 conservative reference occurrences) render every input field without `unknown`; these Pi versions expand only 16/17 target references and ignore reference siblings. - An independent direct-reference control reproduces the actual cutoff: 32 references stay typed; the 33rd renders `zz_extra: unknown`. REA's existing inline fallback restores `zz_extra: string` in both versions. - Pi 1.2.0 renders the sibling fixture's input in 296 characters shared versus 636 inline. The conservative selector can therefore inline earlier than that renderer needs; no field-loss repair or efficiency gain is claimed for the sibling fixture. Pinned renderer sources: [1.1.0](https://github.com/earendil-works/pi/blob/abe508e1b89912adde45528136c3221eb69acdd7/packages/codemode/src/declarations.ts#L255-L267) and [1.2.0](https://github.com/earendil-works/pi/blob/06a466935536f8c9bcacb2d5efc0fc021be1bdff/packages/codemode/src/declarations.ts#L176-L188). This isolates published renderer behavior after SDK conversion; an interactive Pi host/model session remains unverified. **Limits:** the standard `npm run check` stops in the pinned native TypeScript 7 compiler because this host lacks `/proc/self/exe`; a passing native gate is not claimed. A whole-repository format attempt in this environment also fails in untouched HTML workers at `uv_resident_set_memory`; the three changed TypeScript files pass. SDK loopback and isolated renderer checks do not establish an interactive Pi/client session, a full CLI/provider workflow, performance gains or full CI coverage. ## Current-main status Superseded by [5e30969](https://github.com/morluto/rea/commit/5e30969cc5f1255d44ec7a1f1ca8dbac0af5e7c4), which inlines advertised inputs at their projection owner and removes the reference-budget mechanism and tests. This patch no longer applies to current main; resolving its conflicts by restoring the deleted counter would revive a superseded representation. The validation above describes the historical branch, not current-main acceptance. Details: https://github.com/morluto/rea/pull/3920#issuecomment-6201011007 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:46 +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#5965

No description provided.

Rows
Columns