[PR #6899] [CLOSED] fix(sqlite): budget schema names by returned text #7433

New issue

Closed

opened 2026-11-01 21:47:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6899
Author: @gensokyo-dev
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/6888-sqlite-schema-budget-20261023

📝 Commits (1)

7a46b39 fix(sqlite): budget schema names by returned text

📊 Changes

2 files changed (+44 additions, -24 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseInspection.ts (+6 -4)
📝 tests/boundary/sqlite/sqliteDatabaseInspection.test.ts (+38 -20)

📄 Description

Superseded by the merged fix

Maintainer commit 25aaf8928763 independently fixes #6888 on main. The issue is closed. Verified current main 34563fcc9bd92af30668820dcac5635e27181094 against the original UTF-16le/UTF-16be fixtures in both scopes: all four complete results agree byte-for-byte with the repaired inspection facts. The remaining Latin-1 lookup optimization is not needed to resolve the bug; closing this duplicate PR after verification.

On current main, npm run test:focused -- tests/boundary/sqlite/sqliteDatabaseInspection.test.ts tests/acceptance/sqliteDatabaseCli.test.ts passed: 52 tests passed, 2 runtime-inapplicable tests skipped. The complete native-worker, CLI and MCP acceptance workflows were executed.

The original PR CI source job reported seven failures in Frida, IDA, JavaScript and bundle-resource tests; those failures were not diagnosed after the upstream fix superseded this change. They are not reported as passing. Automated Codex review on 7a46b394e7 completed with no findings. Historical branch validation follows.

Summary

Fixes #6888. Whole-database SQLite inventory and detailed schema requests charged internal hexadecimal lookup keys against the 16 MiB JSON reply budget. Valid UTF-16 schemas could consequently fail even when their complete replies fit.

Charge the returned name spelling instead. Keep exact byte identity in reversible Latin-1 lookup keys, avoiding hexadecimal expansion and an unnecessary input-buffer copy. ASCII folding, strict schema decoding, selected-table isolation, final worker JSON validation and existing resource limits remain enforced by their existing owners. CLI and MCP share this worker implementation; no public schema changes or new execution authority. This is a routine fix within the existing SQLite profile; a new maintainer scope decision is not applicable.

Validation performed

Before the fix, the production boundary reproduced output-limit for both UTF-16 encodings; the UTF-8 control passed. The regression extends the existing 48-table capacity case to UTF-8/UTF-16le/UTF-16be and both schema scopes, checking names, declarations, columns and actual serialized reply size.
npm run check — passed, including typecheck, formatting, lint, module boundaries, unused-code and architecture checks.
npm run test:focused -- tests/boundary/sqlite/sqliteDatabaseInspection.test.ts tests/boundary/sqlite/sqliteColumnIdentity.test.ts tests/boundary/sqlite/sqliteDatabaseProvider.test.ts tests/acceptance/sqliteDatabaseCli.test.ts — 87 passed, 2 runtime-inapplicable cases skipped. After retaining the original 48-table capacity, the changed boundary file was rerun: 33 passed, 1 runtime-inapplicable case skipped.
npm run verify:test-discovery — all 2,174 files discovered exactly once.
Real compiled CLI and stdio MCP: the issue's 34-table UTF-16 fixtures now retain complete inventory; CLI also delivers complete detailed schema. The default MCP 10 MiB transport budget still refuses the larger detailed response, with a retained Evidence reference. Exported that bundle and verified every schema fact and original digest against the native inspection. Input hashes remained unchanged.
Real application control: /usr/share/proj/proj.db (36 tables, SHA-256 2cba929271a6c281f5a56805139e4601328e711dfd6e233fcb234c5209b59995) passed native integrity checking and CLI/MCP inventory, detailed schema and Evidence export. Before/after facts, CLI Evidence, MCP catalog and Evidence IDs agreed.

Linux x64, Node 24.22.1 / SQLite 3.53.4, npm 11.16.0; base 5d2c8c2b78d2552da0e13a11d3e2678efb69810d. Windows/macOS were not run locally. Native over-limit, malformed-name, WAL, cancellation and cleanup regressions passed; no synthetic provider substitutes were used for SQLite.

Resource and context measurements

One before/after pair on the same PROJ snapshot with warm OS caches; these are observations, not a stable speedup claim. CLI uses fresh processes. MCP includes discovery, one fresh inventory call, one warm inventory call, detailed schema and Evidence export.

Completed task	Elapsed before → after	CPU before → after	Sampled process-family peak before → after
CLI inventory	2.530 → 2.690 s	3.867 → 4.005 s	435.1 → 446.2 MiB
MCP workflow	14.553 → 13.309 s	19.842 → 18.457 s	969.8 → 962.1 MiB

MCP fresh inventory: 11.925 → 10.857 s; warm inventory: 472.8 → 467.5 ms. Family RSS was sampled from /proc about every 30 ms; short peaks may be missed. Both harnesses completed under a 2 GiB early-stop threshold, without a resource stop. Builds used two checker threads; owned SQLite workers retained their 256 MiB JavaScript heap limit.

The equivalent MCP task used the same 3,150,224-byte catalog and an unchanged 1,166,884-token tokenx heuristic estimate for discovery, arguments, replies and export. This is not actual model usage; prompts, initialization and protocol framing are unmeasured. No token reduction is claimed. The large-name fixture is artificial; no claim about real-world incidence or general analysis speed is made.

Author readiness

Human owner: @gensokyo-dev

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.

_{🔄 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/6899 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6888-sqlite-schema-budget-20261023` --- ### 📝 Commits (1) - [`7a46b39`](https://github.com/morluto/rea/commit/7a46b394e716f447f043061a0d1f99dd85e054b2) fix(sqlite): budget schema names by returned text ### 📊 Changes **2 files changed** (+44 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseInspection.ts` (+6 -4) 📝 `tests/boundary/sqlite/sqliteDatabaseInspection.test.ts` (+38 -20) </details> ### 📄 Description ## Superseded by the merged fix Maintainer commit [25aaf8928763](https://github.com/morluto/rea/commit/25aaf8928763c60f84c4d5bb4f369d976d2d6a92) independently fixes #6888 on main. The issue is closed. Verified current main `34563fcc9bd92af30668820dcac5635e27181094` against the original UTF-16le/UTF-16be fixtures in both scopes: all four complete results agree byte-for-byte with the repaired inspection facts. The remaining Latin-1 lookup optimization is not needed to resolve the bug; closing this duplicate PR after verification. On current main, `npm run test:focused -- tests/boundary/sqlite/sqliteDatabaseInspection.test.ts tests/acceptance/sqliteDatabaseCli.test.ts` passed: 52 tests passed, 2 runtime-inapplicable tests skipped. The complete native-worker, CLI and MCP acceptance workflows were executed. The original PR CI source job reported seven failures in Frida, IDA, JavaScript and bundle-resource tests; those failures were not diagnosed after the upstream fix superseded this change. They are not reported as passing. Automated Codex review on `7a46b394e7` completed with no findings. Historical branch validation follows. ## Summary Fixes #6888. Whole-database SQLite inventory and detailed schema requests charged internal hexadecimal lookup keys against the 16 MiB JSON reply budget. Valid UTF-16 schemas could consequently fail even when their complete replies fit. Charge the returned name spelling instead. Keep exact byte identity in reversible Latin-1 lookup keys, avoiding hexadecimal expansion and an unnecessary input-buffer copy. ASCII folding, strict schema decoding, selected-table isolation, final worker JSON validation and existing resource limits remain enforced by their existing owners. CLI and MCP share this worker implementation; no public schema changes or new execution authority. This is a routine fix within the existing SQLite profile; a new maintainer scope decision is not applicable. ## Validation performed - Before the fix, the production boundary reproduced `output-limit` for both UTF-16 encodings; the UTF-8 control passed. The regression extends the existing 48-table capacity case to UTF-8/UTF-16le/UTF-16be and both schema scopes, checking names, declarations, columns and actual serialized reply size. - `npm run check` — passed, including typecheck, formatting, lint, module boundaries, unused-code and architecture checks. - `npm run test:focused -- tests/boundary/sqlite/sqliteDatabaseInspection.test.ts tests/boundary/sqlite/sqliteColumnIdentity.test.ts tests/boundary/sqlite/sqliteDatabaseProvider.test.ts tests/acceptance/sqliteDatabaseCli.test.ts` — 87 passed, 2 runtime-inapplicable cases skipped. After retaining the original 48-table capacity, the changed boundary file was rerun: 33 passed, 1 runtime-inapplicable case skipped. - `npm run verify:test-discovery` — all 2,174 files discovered exactly once. - Real compiled CLI and stdio MCP: the issue's 34-table UTF-16 fixtures now retain complete inventory; CLI also delivers complete detailed schema. The default MCP 10 MiB transport budget still refuses the larger detailed response, with a retained Evidence reference. Exported that bundle and verified every schema fact and original digest against the native inspection. Input hashes remained unchanged. - Real application control: `/usr/share/proj/proj.db` (36 tables, SHA-256 `2cba929271a6c281f5a56805139e4601328e711dfd6e233fcb234c5209b59995`) passed native integrity checking and CLI/MCP inventory, detailed schema and Evidence export. Before/after facts, CLI Evidence, MCP catalog and Evidence IDs agreed. Linux x64, Node 24.22.1 / SQLite 3.53.4, npm 11.16.0; base `5d2c8c2b78d2552da0e13a11d3e2678efb69810d`. Windows/macOS were not run locally. Native over-limit, malformed-name, WAL, cancellation and cleanup regressions passed; no synthetic provider substitutes were used for SQLite. <details> <summary>Resource and context measurements</summary> One before/after pair on the same PROJ snapshot with warm OS caches; these are observations, not a stable speedup claim. CLI uses fresh processes. MCP includes discovery, one fresh inventory call, one warm inventory call, detailed schema and Evidence export. | Completed task | Elapsed before → after | CPU before → after | Sampled process-family peak before → after | | --- | --- | --- | --- | | CLI inventory | 2.530 → 2.690 s | 3.867 → 4.005 s | 435.1 → 446.2 MiB | | MCP workflow | 14.553 → 13.309 s | 19.842 → 18.457 s | 969.8 → 962.1 MiB | MCP fresh inventory: 11.925 → 10.857 s; warm inventory: 472.8 → 467.5 ms. Family RSS was sampled from `/proc` about every 30 ms; short peaks may be missed. Both harnesses completed under a 2 GiB early-stop threshold, without a resource stop. Builds used two checker threads; owned SQLite workers retained their 256 MiB JavaScript heap limit. The equivalent MCP task used the same 3,150,224-byte catalog and an unchanged 1,166,884-token `tokenx` heuristic estimate for discovery, arguments, replies and export. This is not actual model usage; prompts, initialization and protocol framing are unmeasured. No token reduction is claimed. The large-name fixture is artificial; no claim about real-world incidence or general analysis speed is made. </details> ## Author readiness Human owner: @gensokyo-dev - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:49 +00:00