[PR #3018] [MERGED] test(analysis-view): order the message-cutoff fixture lexicographically #5560

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:07:34 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3018
Author: @morconsultoriadigital-afk
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/analysis-view-message-order-fixture

📝 Commits (1)

da89999 test(analysis-view): order the message-cutoff fixture lexicographically

📊 Changes

1 file changed (+1 additions, -1 deletions)

View changed files

📝 src/domain/analysisView/analysisView.test.ts (+1 -1)

📄 Description

Summary

src/domain/analysisView/analysisView.test.ts fails on main at 9630c8ff4. The
test "does not impose a message-count cutoff below the byte budget" asserts
against generation order while boundedViewMessages returns sorted output, so
the fixture never matched. The assertion, not the implementation, was wrong.

Problem and expected behavior

boundedViewMessages (src/domain/analysisView/analysisViewCoverage.ts:48)
returns uniqueSorted([...priority, ...values]). The fixture generated
`message ${index}` for 100 indices and compared the result to the
generation-ordered array. Lexicographic order places "message 6" between
"message 59" and "message 60", so toEqual always failed.

The byte budget is not involved: each message costs
(9 * 18 + 6) * 3 = 504 wire bytes, so 100 messages total roughly 50 KB
against the 256 KB ANALYSIS_VIEW_MESSAGE_BYTE_LIMIT. The function returns
early through the totalBytes <= ANALYSIS_VIEW_MESSAGE_BYTE_LIMIT branch, and
the intended invariant — no count-based cutoff below the byte budget — holds.

Introduced by 20b802f16, "fix(analysis-view): bound limitations and unknowns
per view" (#2794).

Change and scope

Pad the index to a fixed width so lexicographic and numeric order coincide.
One line, fixture only.

Padding was chosen over sorting the expectation so the test still names the
exact set it expects rather than restating the implementation's ordering.
Production code is intentionally untouched: sorted output is the documented
contract of uniqueSorted, and the other 26 tests in the file depend on it.

Follow the contribution scope.

Contract and boundary impact

Semantic owner and earliest changed stage: test fixture only; no stage changes.
Contribution scope fit and linked maintainer decision: not applicable — test-only fix inside existing boundaries.
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: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects:

Documentation or usability only; no target interaction.

Evidence and regression coverage

Evidence is executed.

Tests added or updated: src/domain/analysisView/analysisView.test.ts — fixture corrected; no test added or removed.
Base reproduction: on 9630c8ff4 without this patch, npm run test:fast reports 1 failed | 6427 passed, failing at analysisView.test.ts:964 with the lexicographic diff. With this patch the same command reports 461 passed (461) / 6428 passed | 7 skipped.
User-visible CLI/MCP output: not applicable.
Remaining proof gaps: only the test:fast projects ran (domain, services, adapters, composition, conformance, evaluation). Boundary, process, MCP, and acceptance projects were not run, nor were the real-provider lanes.

Validation performed

Ubuntu 24.04.5 on WSL2, Node 24.21.0, repo on the Linux filesystem.

npm ci — 401 packages, 0 vulnerabilities.
npm run build — 3/3 turbo tasks successful.
npx vitest run --project domain src/domain/analysisView/analysisView.test.ts — 27 passed.
npm run test:fast — 461 passed (461), 6428 passed | 7 skipped (6435).
npm run lint — 2964 files, 2 errors, 1123 warnings. Both errors (tests/boundary/filesystem/artifactComparison.test.ts:346 and src/dotnet/ManagedCilDecoding.test.ts:248, both max-lines-per-function) reproduce identically on 9630c8ff4 without this patch and are untouched by this diff. The pre-push hook was bypassed for that reason.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not exercised; not reachable from this change.
Package or release metadata impact: none.
Security, privacy, process, or containment review: not applicable — no production code changed.

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 — no issue found for this failure; it appears to have landed with #2794 before CI completed.
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/3018 **Author:** [@morconsultoriadigital-afk](https://github.com/morconsultoriadigital-afk) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/analysis-view-message-order-fixture` --- ### 📝 Commits (1) - [`da89999`](https://github.com/morluto/rea/commit/da89999dda797a0cfadef69703503bd1164b3bbc) test(analysis-view): order the message-cutoff fixture lexicographically ### 📊 Changes **1 file changed** (+1 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisView/analysisView.test.ts` (+1 -1) </details> ### 📄 Description ## Summary `src/domain/analysisView/analysisView.test.ts` fails on `main` at `9630c8ff4`. The test "does not impose a message-count cutoff below the byte budget" asserts against generation order while `boundedViewMessages` returns sorted output, so the fixture never matched. The assertion, not the implementation, was wrong. ## Problem and expected behavior `boundedViewMessages` (`src/domain/analysisView/analysisViewCoverage.ts:48`) returns `uniqueSorted([...priority, ...values])`. The fixture generated `` `message ${index}` `` for 100 indices and compared the result to the generation-ordered array. Lexicographic order places `"message 6"` between `"message 59"` and `"message 60"`, so `toEqual` always failed. The byte budget is not involved: each message costs `(9 * 18 + 6) * 3 = 504` wire bytes, so 100 messages total roughly 50 KB against the 256 KB `ANALYSIS_VIEW_MESSAGE_BYTE_LIMIT`. The function returns early through the `totalBytes <= ANALYSIS_VIEW_MESSAGE_BYTE_LIMIT` branch, and the intended invariant — no count-based cutoff below the byte budget — holds. Introduced by 20b802f16, "fix(analysis-view): bound limitations and unknowns per view" (#2794). ## Change and scope Pad the index to a fixed width so lexicographic and numeric order coincide. One line, fixture only. Padding was chosen over sorting the expectation so the test still names the exact set it expects rather than restating the implementation's ordering. Production code is intentionally untouched: sorted output is the documented contract of `uniqueSorted`, and the other 26 tests in the file depend on it. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). ## Contract and boundary impact - Semantic owner and earliest changed stage: test fixture only; no stage changes. - Contribution scope fit and linked maintainer decision: not applicable — test-only fix inside existing boundaries. - 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: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage Evidence is executed. - Tests added or updated: `src/domain/analysisView/analysisView.test.ts` — fixture corrected; no test added or removed. - Base reproduction: on `9630c8ff4` without this patch, `npm run test:fast` reports `1 failed | 6427 passed`, failing at `analysisView.test.ts:964` with the lexicographic diff. With this patch the same command reports `461 passed (461)` / `6428 passed | 7 skipped`. - User-visible CLI/MCP output: not applicable. - Remaining proof gaps: only the `test:fast` projects ran (domain, services, adapters, composition, conformance, evaluation). Boundary, process, MCP, and acceptance projects were not run, nor were the real-provider lanes. ## Validation performed Ubuntu 24.04.5 on WSL2, Node 24.21.0, repo on the Linux filesystem. - `npm ci` — 401 packages, 0 vulnerabilities. - `npm run build` — 3/3 turbo tasks successful. - `npx vitest run --project domain src/domain/analysisView/analysisView.test.ts` — 27 passed. - `npm run test:fast` — `461 passed (461)`, `6428 passed | 7 skipped (6435)`. - `npm run lint` — `2964 files, 2 errors, 1123 warnings`. Both errors (`tests/boundary/filesystem/artifactComparison.test.ts:346` and `src/dotnet/ManagedCilDecoding.test.ts:248`, both `max-lines-per-function`) reproduce identically on `9630c8ff4` without this patch and are untouched by this diff. The `pre-push` hook was bypassed for that reason. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not exercised; not reachable from this change. - Package or release metadata impact: none. - Security, privacy, process, or containment review: not applicable — no production code changed. ## 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`. - [ ] Related issue is linked — no issue found for this failure; it appears to have landed with #2794 before CI completed. - [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-10-31 01:07:34 +00:00