morluto/rea

Fork 0

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

[PR #6427] [MERGED] test(process): print the progress frames when the sparse-child bound fails #7353

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6427
Author: @CassieFox23
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: fix/5889-progress-frame-diagnostics

📝 Commits (1)

1a92475 test(process): print the progress frames when the sparse-child bound fails

📊 Changes

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

View changed files

📝 tests/boundary/process/processCaptureProgress.test.ts (+4 -1)

📄 Description

TL;DR: A CI test failed with only "expected false to be true", so nobody could see the frame counts. Now the failure prints the progress details. This does not fix the flake, so it says Refs and not Fixes. Thank you, gensokyo-dev, for the careful report.

Summary

Refs #5889. Title: test(process): print the progress frames when the sparse-child bound fails.
The sparse-child test checks that no more than two frames were recorded. It checked this with a bare true/false value. When it failed in CI, the log said only expected false to be true. Now the check prints the progress details when it fails. Thank you, gensokyo-dev, for the report and for saying plainly what was not known.

Problem and expected behavior

The line expect(running.every(({ details }) => details.frames <= 2)).toBe(true) hides the data it checks. A failure shows false and nothing else. The issue asks to keep the actual updates on failure.
Expected: if the bound fails, the output lists each running update. Each one shows elapsed time, frames, samples and interactions. That tells us when the frame count climbs.

Change and scope

The assertion now passes a message to expect, the same expect(value, context) form this file already uses at its exit-code checks. The message is the JSON of all running progress details. The bound itself is unchanged.
This PR says Refs and not Fixes. It adds diagnostics only. The flake is not reproduced and not fixed. Not included: changing the bound, dropping frames, or touching any code under src/. The issue asks us not to relax the test just to get green CI.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage:
tests/boundary/process/processCaptureProgress.test.ts, the sparse-child assertion at the end of the test. Test code only.
Contribution scope fit and linked maintainer decision:
Not applicable. A diagnostic change inside an existing test.
CLI and MCP/tool-catalog contract:
None.
Provider, bridge, target-format, or platform compatibility:
None. Test-only change, run on Node 22, 24 and 26 on Linux x64.
Evidence, artifact, provenance, or reconstruction contract:
None.
Target selection, authority, upstream behavior, and cleanup details (if applicable):
Not applicable. The test already runs its own small Node child. This change adds no new target or process.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs):
None. Test file only.

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:
tests/boundary/process/processCaptureProgress.test.ts (one assertion, +4/-1 lines). No new test.
Base reproduction or other evidence:
Executed on main (ed069bb9d), Node 24.18.0. I could not reproduce the CI flake, so I forced a failure. I changed <= 2 to <= 0 on main's assertion. Then I did the same on this PR's assertion. These forced runs show the output format only. The numbers come from a normal passing run, not from the flake.

Before (main), trimmed:
```
AssertionError: expected false to be true // Object.is equality
- Expected
+ Received
- true
+ false
 ❯ tests/boundary/process/processCaptureProgress.test.ts:276:65
```
After (this PR), trimmed:
```
AssertionError: running progress details: [{"elapsed_ms":161,"frames":0,"samples":0,"interactions":0},{"elapsed_ms":263,"frames":1,"samples":1,"interactions":0},{"elapsed_ms":383,"frames":1,...},...,{"elapsed_ms":1788,"frames":1,"samples":1,"interactions":0}]: expected false to be true // Object.is equality
 ❯ tests/boundary/process/processCaptureProgress.test.ts:279:7
```
That run had 16 running updates: frames 0 once, then 1 for the rest, from 161 ms to 1788 ms. The <= 0 edits were reverted. The commit holds only the <= 2 assertion with the message.
User-visible CLI/MCP output (if applicable):
Not applicable.
Remaining proof gaps:
The flake is not reproduced and not fixed. The message wording was checked only with the forced threshold, not on a real CI failure. The message prints all running updates, not only the ones over the bound. If that is too long, expect(running.filter(({ details }) => details.frames > 2)).toEqual([]) is a shorter option.

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

npm run test:focused -- tests/boundary/process/processCaptureProgress.test.ts with this change on Node 22.23.1 — 7 passed
same command with this change on Node 24.18.0 — 7 passed
same command with this change on Node 26.11.1 — 7 passed
same command, forced failure (<= 0) on main's assertion, Node 24.18.0 — 1 failed, 6 passed; prints only expected false to be true
same command, forced failure (<= 0) on this PR, Node 24.18.0 — 1 failed, 6 passed; prints the details
npm run typecheck — exit 0
npm run lint — exit 0 (0 errors; warnings repo-wide, none on the touched file)
npm run format:check — exit 0
No full suite, test:fast or check:fast was run.

Compatibility, safety, and release

Breaking changes or migration steps:
None.
Real Hopper/Ghidra, browser, or OS coverage:
Not applicable. Linux x64 only for the test run.
Package or release metadata impact:
None.
Security, privacy, process, or containment review:
None needed. The change only adds text to a failure message. No new process or network use.

Author readiness

Human owner: @CassieFox23

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.

Test setup

Box: Nobara 44 (Fedora 44 base), Linux 7.2.11
CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
RAM: 32 GB DDR4-3600
Node: 22.23.1, 24.18.0, 26.11.1
Head: 63d704012. Base: ed069bb9d.

🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊

_{🔄 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/6427 **Author:** [@CassieFox23](https://github.com/CassieFox23) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/5889-progress-frame-diagnostics` --- ### 📝 Commits (1) - [`1a92475`](https://github.com/morluto/rea/commit/1a92475fea8c2fce864ce8e08f1da83fc99f457d) test(process): print the progress frames when the sparse-child bound fails ### 📊 Changes **1 file changed** (+4 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/process/processCaptureProgress.test.ts` (+4 -1) </details> ### 📄 Description **TL;DR:** A CI test failed with only "expected false to be true", so nobody could see the frame counts. Now the failure prints the progress details. This does not fix the flake, so it says Refs and not Fixes. Thank you, gensokyo-dev, for the careful report.  ## Summary  Refs #5889. Title: `test(process): print the progress frames when the sparse-child bound fails`. The sparse-child test checks that no more than two frames were recorded. It checked this with a bare true/false value. When it failed in CI, the log said only `expected false to be true`. Now the check prints the progress details when it fails. Thank you, gensokyo-dev, for the report and for saying plainly what was not known. ## Problem and expected behavior  The line `expect(running.every(({ details }) => details.frames <= 2)).toBe(true)` hides the data it checks. A failure shows `false` and nothing else. The issue asks to keep the actual updates on failure. Expected: if the bound fails, the output lists each running update. Each one shows elapsed time, frames, samples and interactions. That tells us when the frame count climbs. ## Change and scope  The assertion now passes a message to `expect`, the same `expect(value, context)` form this file already uses at its exit-code checks. The message is the JSON of all running progress details. The bound itself is unchanged. This PR says Refs and not Fixes. It adds diagnostics only. The flake is not reproduced and not fixed. Not included: changing the bound, dropping frames, or touching any code under `src/`. The issue asks us not to relax the test just to get green CI. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact  - Semantic owner and earliest changed stage: `tests/boundary/process/processCaptureProgress.test.ts`, the sparse-child assertion at the end of the test. Test code only. - Contribution scope fit and linked maintainer decision: Not applicable. A diagnostic change inside an existing test. - CLI and MCP/tool-catalog contract: None. - Provider, bridge, target-format, or platform compatibility: None. Test-only change, run on Node 22, 24 and 26 on Linux x64. - Evidence, artifact, provenance, or reconstruction contract: None. - Target selection, authority, upstream behavior, and cleanup details (if applicable): Not applicable. The test already runs its own small Node child. This change adds no new target or process. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): None. Test file only. 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: `tests/boundary/process/processCaptureProgress.test.ts` (one assertion, +4/-1 lines). No new test. - Base reproduction or other evidence: Executed on main (ed069bb9d), Node 24.18.0. I could not reproduce the CI flake, so I forced a failure. I changed `<= 2` to `<= 0` on main's assertion. Then I did the same on this PR's assertion. These forced runs show the output format only. The numbers come from a normal passing run, not from the flake. Before (main), trimmed: ``` AssertionError: expected false to be true // Object.is equality - Expected + Received - true + false ❯ tests/boundary/process/processCaptureProgress.test.ts:276:65 ``` After (this PR), trimmed: ``` AssertionError: running progress details: [{"elapsed_ms":161,"frames":0,"samples":0,"interactions":0},{"elapsed_ms":263,"frames":1,"samples":1,"interactions":0},{"elapsed_ms":383,"frames":1,...},...,{"elapsed_ms":1788,"frames":1,"samples":1,"interactions":0}]: expected false to be true // Object.is equality ❯ tests/boundary/process/processCaptureProgress.test.ts:279:7 ``` That run had 16 running updates: frames 0 once, then 1 for the rest, from 161 ms to 1788 ms. The `<= 0` edits were reverted. The commit holds only the `<= 2` assertion with the message. - User-visible CLI/MCP output (if applicable): Not applicable. - Remaining proof gaps: The flake is not reproduced and not fixed. The message wording was checked only with the forced threshold, not on a real CI failure. The message prints all running updates, not only the ones over the bound. If that is too long, `expect(running.filter(({ details }) => details.frames > 2)).toEqual([])` is a shorter option. 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  - `npm run test:focused -- tests/boundary/process/processCaptureProgress.test.ts` with this change on Node 22.23.1 — 7 passed - same command with this change on Node 24.18.0 — 7 passed - same command with this change on Node 26.11.1 — 7 passed - same command, forced failure (`<= 0`) on main's assertion, Node 24.18.0 — 1 failed, 6 passed; prints only `expected false to be true` - same command, forced failure (`<= 0`) on this PR, Node 24.18.0 — 1 failed, 6 passed; prints the details - `npm run typecheck` — exit 0 - `npm run lint` — exit 0 (0 errors; warnings repo-wide, none on the touched file) - `npm run format:check` — exit 0 - No full suite, `test:fast` or `check:fast` was run. ## Compatibility, safety, and release  - Breaking changes or migration steps: None.  - Real Hopper/Ghidra, browser, or OS coverage: Not applicable. Linux x64 only for the test run. - Package or release metadata impact: None. - Security, privacy, process, or containment review: None needed. The change only adds text to a failure message. No new process or network use. ## Author readiness  Human owner: @CassieFox23 - [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  - [ ] 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. <details> <summary>Test setup</summary> - Box: Nobara 44 (Fedora 44 base), Linux 7.2.11 - CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads - RAM: 32 GB DDR4-3600 - Node: 22.23.1, 24.18.0, 26.11.1 - Head: 63d704012. Base: ed069bb9d. </details> 🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊 --- <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:16 +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#7353

No description provided.

Rows
Columns