[PR #5013] [CLOSED] test(hopper): guard against string record reprojection in literal search #6732

Closed
opened 2026-11-01 21:42:39 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5013
Author: @RishabhhR
Created: 10/18/2026
Status: ❌ Closed

Base: main ← Head: fix/hopper-string-search-selected-projection


📝 Commits (3)

  • 216feb5 fix(hopper): project only matched string records in literal search
  • 352f2d7 test(hopper): order recorded string projections numerically
  • 80c74de Merge morluto/rea main into fix/hopper-string-search-selected-projection

📊 Changes

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

View changed files

📝 tests/boundary/providers/hopper/hopperStringSearch.test.ts (+11 -0)
📝 tests/fixtures/hopperStringSearchProbe.py (+13 -1)

📄 Description

Summary

Adds a regression check for the string-search projection cost reported in #5010. That issue was fixed by #5017, which keeps decoded string records in the Hopper search inventory and matches them directly. This PR changes no production code; it guards that behavior so a later change cannot silently reintroduce a full record re-projection on every positive search.

(This PR originally carried its own bridge fix; after #5017 landed, the conflict was resolved by taking main's implementation, leaving only the test.)

Problem and expected behavior

Before #5017, every positive literal search_strings query rebuilt the complete string record map after matching, even for a single match. The existing string-search tests checked result correctness only, so they could not detect that cost. Expected: the record inventory is built once for an unchanged document; later literal searches, positive or negative, do no further record projection.

Change and scope

  • tests/fixtures/hopperStringSearchProbe.py wraps _string_inventory to record each projection (full, or the numerically ordered selected addresses) per query.
  • tests/boundary/providers/hopper/hopperStringSearch.test.ts adds "builds the record inventory once and never reprojects it for matches": the cold first query performs exactly one full projection, and each of the 13 later queries (sparse, broad, absent, case-sensitive, repeated) performs none.

No bridge, contract, or runtime change. Contribution scope: test-only; maintainer scope decision not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: none changed; the test observes _string_inventory (Hopper bridge owner).
  • Contribution scope fit and linked maintainer decision: not applicable (test-only).
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: the projection-count test above; the probe records projections per query.
  • Base reproduction or other evidence: executed. Running the probe against the pre-#5017 bridge (c12d9eda8): results are still correct, but the cold query projects the full inventory twice and 11 of 13 warm queries re-project it (the two absent-pattern queries do not), so the new test fails there. Against current main it passes.
  • User-visible CLI/MCP output (if applicable): unchanged.
  • Remaining proof gaps: simulated Hopper-shaped document only; no real Hopper run. Latency/CPU/memory/token impact is not measured here (see #5017's recorded measurements).

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • python3 tests/fixtures/hopperStringSearchProbe.py <bridge from c12d9eda8> — records correct; 1 extra full projection on the cold query, re-projection on 11 of 13 warm queries.
  • npx vitest run tests/boundary/providers/hopper on 80c74de95 (merged with main at 63c970812) — 25 files passed (159 passed, 6 skipped).
  • npx prettier --write and npx oxlint on the changed test — clean.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: none; real Hopper not run.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: test-only; no new process, network or file effects.

Author readiness

Human owner: @RishabhhR

  • 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.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 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/5013 **Author:** [@RishabhhR](https://github.com/RishabhhR) **Created:** 10/18/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/hopper-string-search-selected-projection` --- ### 📝 Commits (3) - [`216feb5`](https://github.com/morluto/rea/commit/216feb56b3cf703dc1d2db3e0a1dbfabe71e88ef) fix(hopper): project only matched string records in literal search - [`352f2d7`](https://github.com/morluto/rea/commit/352f2d702b36f92f4ac5e15856c16e65b4fef412) test(hopper): order recorded string projections numerically - [`80c74de`](https://github.com/morluto/rea/commit/80c74de95e05cf736921ed78e2be8f62a8945b2a) Merge morluto/rea main into fix/hopper-string-search-selected-projection ### 📊 Changes **2 files changed** (+24 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/providers/hopper/hopperStringSearch.test.ts` (+11 -0) 📝 `tests/fixtures/hopperStringSearchProbe.py` (+13 -1) </details> ### 📄 Description ## Summary Adds a regression check for the string-search projection cost reported in #5010. That issue was fixed by #5017, which keeps decoded string records in the Hopper search inventory and matches them directly. This PR changes no production code; it guards that behavior so a later change cannot silently reintroduce a full record re-projection on every positive search. (This PR originally carried its own bridge fix; after #5017 landed, the conflict was resolved by taking `main`'s implementation, leaving only the test.) ## Problem and expected behavior Before #5017, every positive literal `search_strings` query rebuilt the complete string record map after matching, even for a single match. The existing string-search tests checked result correctness only, so they could not detect that cost. Expected: the record inventory is built once for an unchanged document; later literal searches, positive or negative, do no further record projection. ## Change and scope - `tests/fixtures/hopperStringSearchProbe.py` wraps `_string_inventory` to record each projection (full, or the numerically ordered selected addresses) per query. - `tests/boundary/providers/hopper/hopperStringSearch.test.ts` adds "builds the record inventory once and never reprojects it for matches": the cold first query performs exactly one full projection, and each of the 13 later queries (sparse, broad, absent, case-sensitive, repeated) performs none. No bridge, contract, or runtime change. Contribution scope: test-only; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: none changed; the test observes `_string_inventory` (Hopper bridge owner). - Contribution scope fit and linked maintainer decision: not applicable (test-only). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [x] Contact network services. - [x] Recover protected code or work around protections. - [x] Documentation or usability only; no target interaction. - [x] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: the projection-count test above; the probe records projections per query. - Base reproduction or other evidence: executed. Running the probe against the pre-#5017 bridge (`c12d9eda8`): results are still correct, but the cold query projects the full inventory twice and 11 of 13 warm queries re-project it (the two absent-pattern queries do not), so the new test fails there. Against current `main` it passes. - User-visible CLI/MCP output (if applicable): unchanged. - Remaining proof gaps: simulated Hopper-shaped document only; no real Hopper run. Latency/CPU/memory/token impact is not measured here (see #5017's recorded measurements). For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `python3 tests/fixtures/hopperStringSearchProbe.py <bridge from c12d9eda8>` — records correct; 1 extra full projection on the cold query, re-projection on 11 of 13 warm queries. - `npx vitest run tests/boundary/providers/hopper` on `80c74de95` (merged with `main` at `63c970812`) — 25 files passed (159 passed, 6 skipped). - `npx prettier --write` and `npx oxlint` on the changed test — clean. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none; real Hopper not run. - Package or release metadata impact: none. - Security, privacy, process, or containment review: test-only; no new process, network or file effects. ## Author readiness Human owner: @RishabhhR - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 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-01 21:42:39 +00:00
Sign in to join this conversation.
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#6732
No description provided.