[PR #7016] [MERGED] test(javascript): bound route scan work independently of timing #7576

New issue

Closed

opened 2026-11-03 13:38:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:38:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7016
Author: @gensokyo-dev
Created: 10/24/2026
Status: ✅ Merged
Merged: 10/24/2026
Merged by: @gensokyo-dev

Base: main ← Head: test/route-scan-cost-20261024

📝 Commits (1)

5227b2f test(javascript): bound route scan work independently of timing

📊 Changes

2 files changed (+76 additions, -9 deletions)

View changed files

📝 src/domain/javascript/javascriptRouteScan.test.ts (+65 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.routeProperties.test.ts (+11 -9)

📄 Description

Problem and outcome

Main's full qualification failed because the repeated route-property test took 2281 ms under coverage, exceeding its hard-coded 2000 ms assertion. The production scan already caches each object's shape. On fresh base 1184819cb, three local coverage runs also failed solely on that assertion (2791–2874 ms).

Replace the elapsed-time proxy with a deterministic work bound at the real route-scan owner. Real parsed AST keys retain their values; getters count reads only while scan.inspect runs. Two distinct records check both candidate and router classification, with at most one record-key scan beyond reading each property. The full source-analysis case retains exact endpoint values, interpretation and order with 256 repeated keys instead of 20,000. No runtime behavior, deadline, resource cap or retry policy changes.

Executed validation

Linux x64, Node 24.22.1 / npm 11.16.0, two CPU affinity:

Focused tests: 262 passed. Expanded static-analysis/route suite: 1070 passed across 38 files, including route capacity cases (10.10 s wall / 16.44 s family CPU / 1336.3 MiB sampled family RSS).
Final npm run check and git diff --check: passed (39.36 s wall / 66.88 s CPU / 2362.8 MiB). An initial static check caught an exact-optional test projection type mismatch; it was corrected before this successful gate.
Negative control: bypassing only the production shape-cache lookup makes the new assertion fail: 33,410 key reads exceed the 514-read linear bound. Production source restored; only two test files differ.
Three fresh-process coverage runs of both affected files: baseline 261 tests, one failure each run; updated 262 passed each run. Median complete task: 6.394 → 4.597 s wall, 9.052 → 6.402 s CPU; maximum sampled family RSS 704.5 → 567.6 MiB. This measures test-suite cost, including the smaller fixture and added work oracle; it is not an analysis throughput claim. Each process has a cold module graph; OS/file caches were uncontrolled, and warm-process reuse was not measured.

The original 20,000-key source workflow completed and met its correctness assertions before failing its timer in all three baseline runs. The retained source workflow and owner tests use real parsers/analysis, not fabricated provider results. This test-only change makes no new real-provider claim. Agent tokens were not measured; runtime payloads are unchanged. RSS sampling at 100 ms sums descendants, can double-count shared pages and miss short/reparented children, and is not a hard aggregate cap. Tests had a 3 GiB sampled stop and timeout; no OOM occurred.

The full CI run completed with 32,576 portable tests passed, two failed, 233 skipped. Both changed files passed under coverage; the failures are the unchanged Web contract assertions described below. The Windows lane separately failed the same two assertions plus three native-loader warning assertions. Static/deep static/docs, all real-provider lanes, reverse-tools quality, Apple E2E and all four native package baselines passed. This CI run is a failure, not a green qualification. Greptile reviewed the exact head without findings; Codex review remains pending after more than 25 minutes and is not represented as completed. Separately, the executed base 1184819cb main CI failed two unchanged webScriptManifestPaths assertions: main's 4f212c6a3 classifies manifest-derived Windows path failures as artifact_operation_failed / reason: path, while those tests still expect caller-input validation errors. Neither the producer nor those tests is changed by this route-work PR; repair will be isolated after this issue. The full Windows lane additionally exposed three unchanged public-runner stderr assertions: the previously merged native loader emits Node's stripTypeScriptTypes ExperimentalWarning. Exit status and argument bytes remain correct. This will also be repaired in a separate follow-up, preserving target diagnostics. These five assertion failures are recorded as failures, not successful qualification.

Author readiness

Human owner: @gensokyo-dev

The owner explicitly delegated readiness declarations and agent-managed review. Inspection and execution were performed by Codex; no separate human inspection is claimed.

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/7016 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/24/2026 **Status:** ✅ Merged **Merged:** 10/24/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `test/route-scan-cost-20261024` --- ### 📝 Commits (1) - [`5227b2f`](https://github.com/morluto/rea/commit/5227b2fec6b1972aa7005dddf69b40c080be7f3c) test(javascript): bound route scan work independently of timing ### 📊 Changes **2 files changed** (+76 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptRouteScan.test.ts` (+65 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.routeProperties.test.ts` (+11 -9) </details> ### 📄 Description ## Problem and outcome [Main's full qualification](https://github.com/morluto/rea/actions/runs/40102893830/job/120898772833) failed because the repeated route-property test took 2281 ms under coverage, exceeding its hard-coded 2000 ms assertion. The production scan already caches each object's shape. On fresh base `1184819cb`, three local coverage runs also failed solely on that assertion (2791–2874 ms). Replace the elapsed-time proxy with a deterministic work bound at the real route-scan owner. Real parsed AST keys retain their values; getters count reads only while `scan.inspect` runs. Two distinct records check both candidate and router classification, with at most one record-key scan beyond reading each property. The full source-analysis case retains exact endpoint values, interpretation and order with 256 repeated keys instead of 20,000. No runtime behavior, deadline, resource cap or retry policy changes. ## Executed validation Linux x64, Node 24.22.1 / npm 11.16.0, two CPU affinity: - Focused tests: **262 passed**. Expanded static-analysis/route suite: **1070 passed across 38 files**, including route capacity cases (10.10 s wall / 16.44 s family CPU / 1336.3 MiB sampled family RSS). - Final `npm run check` and `git diff --check`: passed (39.36 s wall / 66.88 s CPU / 2362.8 MiB). An initial static check caught an exact-optional test projection type mismatch; it was corrected before this successful gate. - Negative control: bypassing only the production shape-cache lookup makes the new assertion fail: **33,410 key reads exceed the 514-read linear bound**. Production source restored; only two test files differ. - Three fresh-process coverage runs of both affected files: baseline **261 tests, one failure each run**; updated **262 passed each run**. Median complete task: **6.394 → 4.597 s wall**, **9.052 → 6.402 s CPU**; maximum sampled family RSS **704.5 → 567.6 MiB**. This measures test-suite cost, including the smaller fixture and added work oracle; it is not an analysis throughput claim. Each process has a cold module graph; OS/file caches were uncontrolled, and warm-process reuse was not measured. The original 20,000-key source workflow completed and met its correctness assertions before failing its timer in all three baseline runs. The retained source workflow and owner tests use real parsers/analysis, not fabricated provider results. This test-only change makes no new real-provider claim. Agent tokens were not measured; runtime payloads are unchanged. RSS sampling at 100 ms sums descendants, can double-count shared pages and miss short/reparented children, and is not a hard aggregate cap. Tests had a 3 GiB sampled stop and timeout; no OOM occurred. The [full CI run](https://github.com/morluto/rea/actions/runs/40105743147) completed with **32,576 portable tests passed, two failed, 233 skipped**. Both changed files passed under coverage; the failures are the unchanged Web contract assertions described below. The Windows lane separately failed the same two assertions plus three native-loader warning assertions. Static/deep static/docs, all real-provider lanes, reverse-tools quality, Apple E2E and all four native package baselines passed. **This CI run is a failure, not a green qualification.** Greptile reviewed the exact head without findings; Codex review remains pending after more than 25 minutes and is not represented as completed. Separately, the executed base `1184819cb` [main CI](https://github.com/morluto/rea/actions/runs/40105159162/job/120906282001) failed two unchanged `webScriptManifestPaths` assertions: main's `4f212c6a3` classifies manifest-derived Windows path failures as `artifact_operation_failed` / `reason: path`, while those tests still expect caller-input validation errors. Neither the producer nor those tests is changed by this route-work PR; repair will be isolated after this issue. The full Windows lane additionally exposed three unchanged public-runner stderr assertions: the previously merged native loader emits Node's `stripTypeScriptTypes` ExperimentalWarning. Exit status and argument bytes remain correct. This will also be repaired in a separate follow-up, preserving target diagnostics. These five assertion failures are recorded as failures, not successful qualification. ## Author readiness Human owner: @gensokyo-dev The owner explicitly delegated readiness declarations and agent-managed review. Inspection and execution were performed by Codex; no separate human inspection is claimed. - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-03 13:38:49 +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#7576

No description provided.

Rows
Columns