morluto/rea

Fork 0

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

reporting gaps and performance follow-ups #732

New issue

Open

opened 2026-10-15 18:27:46 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-15 18:27:46 +00:00

Owner

Originally created by @N0zoM1z0 on GitHub (Oct 12, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2190

Originally assigned to: @N0zoM1z0 on GitHub.

Implementation progress (2026-10-12)

The initial audit below is retained as historical evidence. The verified
selection/cache/reporting/installer/README/Python gaps are now addressed by
merged #2131 and #2210. Follow-up #2238 was assigned to N0zoM1z0 and closed
by merged #2242 (d96550f2): concise static feedback with complete diagnostic
artifacts, unchanged lint/duplicate failure gates, portable unambiguous clone
locations, and cached lint ownership for all observed source roots.

The final PR passed all 33 checks: 946 files / 9,748 cases, 9,662 passed,
86 skipped, zero failed. Its actual static log has 398 lines / 29,093 bytes,
and the complete artifact retains 911 lint diagnostics and 1,292 clones.
A real website syntax error previously passed cached check:fast but failed
fresh lint; the corrected cache now rejects it. The independent live
checker-inventory/Turbo-input regression fails on the old ownership.

Slow-family and queue follow-ups were investigated rather than assumed:
three complete 99-case runs all passed but showed substantial timing
variation; the explicit live-definition MCP prototype improved cold-request
cost but did not establish stable full-family speedup. It was restored and
no additional test refactor is merged. Three Actions samples show queue
medians from 4 to 49 seconds and maxima from 75 to 199 seconds; these do not
prove a runner/account capacity cause or justify reducing platform/shard
coverage. Those uncertain performance items remain open; no CI-wide speed
percentage is claimed. Current work stops after this evidence-backed merge.

Problem and scope

The CI/test audit found several reproducible verification gaps and a failure-reporting problem. It also found substantial CI cost, but the evidence does not support deleting large parts of the test suite or treating every broad lane selection as a bug.

This issue records the reviewed findings, reproduction steps, measured costs, and acceptance criteria. Correctness findings and performance opportunities are separated below so that optimization does not silently remove coverage.

Baseline: main at 5190843f8f09ac17cbf79561bd1f708c509cda78, verified against the remote on 2026-10-12. Tests used the repository-pinned Node 24.18.0, npm 11.16.0, and Python 3.12 for the website workflow. The installer reproduction used GNU Bash 5.2.15 on Linux.

The initial full Linux test run was on 672bdea21; the two subsequent main commits changed managed CLI schema handling and uninstall output, with no changes to CI, the planner, Turbo, Vitest, or the installer. Both changed test files were rerun on 5190843f8 and all 11 cases passed. The fault-injection and selection probes below were performed against 5190843f8 in isolated temporary checkouts. Project source and workflow files have not been changed as part of this investigation.

Related work: #2131, ci: narrow native-host lane selection and reuse build caches, is open/unmerged at d2e75e22115621cc0a2a89aa24904635fc93134f. Its actual planner was tested as a comparison. Findings already addressed by that PR are identified explicitly; this issue should not create a competing implementation.

Reviewed findings

Finding	Evidence and status	Suggested priority
README-only changes bypass an existing command-parity test	Real README mutation: the existing test fails twice, while the selected docs and website checks pass	P1
Native test-only changes do not select their host lane	Actual main planner reproduces the omission; the planner in #2131 selects the missing host lane	P1; track through #2131
Failed shards prevent merging otherwise usable diagnostics	Observed in Actions; controlled failing shard still produces a mergeable blob, JSON and JUnit	P1
Installer prefix test assumes Bash status/diagnostic behavior that is not universal	Existing test fails again on current main; a minimal Bash program independently reproduces status 0 without executing the guarded body	P1
Cached local full gates omit a real README input and can hide changed host dependencies	Existing README test rejects an actual repository edit, but the cached task passes twice; independent external-dependency injection gives the same result	P1 for missing repository inputs; P2 for host-validation policy
Existing LLDB producer unit tests are outside the automated CI inventory	Nine existing Python tests pass in 0.395s; no invocation in the current workflow/script graph	P2; coverage gap, not a demonstrated producer defect

1. README-only PRs bypass the existing onboarding command-parity guard

The README classification branch selects docs and, for the relevant README paths, website, then continues without selecting code. The existing readmeConsistency.test.ts therefore does not run on these PRs. That test compares translated shell examples with the English examples and checks the installation MCP example.

This is an actual routing gap rather than an assumption that docs checks are weak. In an isolated checkout, I replaced the first fenced Bash example in README_zh.md with rea-command-that-does-not-exist --json:

Check	Original README	Mutated README	Restored README
`npm run test:local -- tests/conformance/readmeConsistency.test.ts`	Pass, 2 tests	Fail twice, explicitly naming `README_zh.md`	Pass
`npm run docs:check`	—	Pass; the first run executed all six tasks rather than relying on a cache hit	—
Python 3.12 `scripts/website.py check`	—	Pass, 97.08s	—
Planner for a README-only diff	—	`code=false`, `docs=true`, `website=true`	—

Reproduce in a disposable checkout with locked dependencies installed:

npm run test:local -- tests/conformance/readmeConsistency.test.ts
python3 - <<'PY'
from pathlib import Path
import re
p = Path('README_zh.md')
p.write_text(re.sub(r'```bash\s*([\s\S]*?)```',
    '```bash\nrea-command-that-does-not-exist --json\n```',
    p.read_text(), count=1))
PY
npm run test:local -- tests/conformance/readmeConsistency.test.ts
npm run docs:check
# Use Python 3.12 with website/requirements.txt installed:
python3 scripts/website.py check

The second test command fails, while both content pipelines accept the mutation. The oracle here is command parity; the existing test is not a complete semantic validator of every CLI example.

Expected change: run this small existing guard in the docs lane, or move its relevant contract into the docs validation owner. Selecting the entire Linux suite for a translated README edit is unnecessary to close this particular gap.

Acceptance: the mutation above fails a README-only PR; the restored README passes; unrelated translated prose does not trigger the complete runtime/provider matrix. If #2141 moves translated READMEs, update both ownership and this guard together.

2. Native test-only edits can produce a green PR without exercising the edited host-specific case

The main planner's generic test-file early return happens before native ownership classification.

Two concrete existing paths demonstrate the problem:

Only changed path	Main planner	Relevant host behavior	Planner from #2131
`tests/boundary/filesystem/windowsNpxRegistration.test.ts`	`code=true`, `package=false`, `apple=false`	Windows-only registration cases skip on Linux; the Windows curated job requires `package=true`	`package=true`, `runtime=true`
`tests/boundary/cli/nativePlistCli.test.ts`	`code=true`, `apple=false`, `package=false`	Its suites skip outside Darwin; the Apple job requires `apple=true`	`apple=true`

Sources: Windows platform condition, Darwin platform condition, Windows job and curated test, Apple job and test.

I exercised the actual planner against small temporary Git repositories containing a base commit and exactly one changed path. Each classification was repeated and compared for equality. Controls included an ordinary domain test, a README-only change, an unowned runtime source, and CI_FULL=true. Ordinary tests still selected the Linux code lane; the full override selected every lane.

Equivalent reproduction in a disposable checkout:

REA_REVIEW_BASE=$(git rev-parse HEAD)
printf '\n// selection probe\n' >> tests/boundary/cli/nativePlistCli.test.ts
git -c user.name='CI review' -c user.email='ci-review@example.invalid' \
  commit -am 'test: probe native test selection'
GITHUB_EVENT_NAME=pull_request BASE_SHA="$REA_REVIEW_BASE" \
  HEAD_SHA=HEAD CI_FULL=false node scripts/ci/plan.mjs

Repeat with the Windows path. These are test-selection reproductions; no macOS/Windows behavior is claimed to have been executed on Linux.

Expected change / acceptance: use the owner-aware fix in #2131; retain deterministic planner regressions asserting that a changed platform-specific test selects its executing host. Preserve ordinary source-test feedback and the explicit full-baseline override. Main pushes already select a full baseline, but that does not repair missing PR feedback.

3. A failing shard blocks report merging before diagnosis can be collected

The aggregate test job first requires all shards to succeed. Checkout, dependency installation, blob download, and merging then require success(). When a shard fails, those steps are skipped. The final always() upload runs without the merged files and fails as well.

This happened in run 38224038323, aggregate job 114730238104, and also in #2131's run 38234806511, aggregate job 114764631340:

Require successful test shards               failure
Checkout / setup / npm ci / blob download    skipped
Merge coverage, JUnit, and timing reports    skipped
Upload test-results                          failure

This follows GitHub's documented status-check semantics. Individual shard logs/blobs remain available; the loss is the merged report and centralized diagnosis, not all evidence of the failure.

To verify that merging failed-test blobs is actually useful, I added one passing and one deliberately failing temporary Vitest spec, then ran the real shard and merge scripts:

npm run test:ci:shard:run -- \
  src/domain/ciReviewControl.test.ts src/domain/ciReviewFailure.test.ts --shard=1/2
npm run test:ci:shard:run -- \
  src/domain/ciReviewControl.test.ts src/domain/ciReviewFailure.test.ts --shard=2/2
npm run test:ci:merge -- \
  --coverage.reportsDirectory=coverage \
  --coverage.reporter=json-summary \
  --reporter=json --reporter=junit \
  --outputFile.json=test-results.json --outputFile.junit=test-results.xml

The temporary specs respectively used it('control', () => expect(true).toBe(true)) and it('controlled failure', () => { throw new Error('controlled-shard-failure'); }), with imports from Vitest.

Observed results:

Shard 1 exited 0; shard 2 exited 1. Both wrote blobs.
Merge exited 1 and wrote valid JSON/JUnit reporting two cases: one passed, one failed.
The tiny fixture also failed normal aggregate coverage floors, as expected; this was not a coverage-performance benchmark.
The actual scripts/ci/required.mjs rejected a failed-shard job set and accepted the all-success control.

Expected change: collect available non-cancelled shard diagnostics before the final success gate. Keep failure/missing-shard/coverage-floor enforcement in the result contract. A report from incomplete shards must be marked incomplete and must never become the successful timing-history baseline.

Acceptance: a controlled failing shard still yields a merged test-results artifact and a failing required check. Missing blobs, cancelled jobs, and failure before Vitest starts are handled explicitly. Do not simply add continue-on-error and make the gate green.

This is distinct from the historical coverage-temp ENOENT in closed #150.

4. The exhaustive installer prefix test has an incorrect Bash oracle on this host

The prefix test starts one Bash process per byte prefix of install.sh: 7,537 subprocesses for the current 7,536-byte file, in batches of 16. It requires every incomplete body prefix to exit nonzero and match syntax error|unexpected EOF.

On Bash 5.2.15, a malformed/truncated [[ ... ]] can report a parsing error while returning status 0. Other prefixes report unexpected token 'EOF' in conditional command, which is also outside the test's regex. A minimal independent reproduction is:

python3 - <<'PY'
import subprocess
p = subprocess.run(['/bin/bash', '-s'],
    input='{\nprintf "EXECUTED\\n"\nif [[ "$verbose" == true ]',
    text=True, capture_output=True)
print('status:', p.returncode)
print('stdout:', repr(p.stdout))
print('stderr:', repr(p.stderr))
PY

Observed: status: 0, empty stdout, and syntax error in conditional expression. The sentinel in the brace group does not execute. This result was repeated under C and C.UTF-8. Positive controls showed a balanced group executes the sentinel, an ordinary incomplete group exits nonzero, and removing the guard lets the sentinel execute before the parse error.

The existing production-boundary test was rerun through the supported command:

npm run test:focused -- tests/boundary/filesystem/installScript.test.ts

Current-main result: 29 cases passed, one failed, 26.40s including command setup / 25.35s Vitest. The same prefix case failed during the initial full suite, taking 30.23s. Recorded failures concerned exit status and diagnostic wording, rather than observed early stdout execution.

Interpretation: the shell test is genuinely nonportable on this host. Separately, a truncated installer pipeline returning status 0 can mislead a caller even if it makes no changes. The independent reproduction does not prove that npm ran or that the brace guard leaked environment data. CI's current Ubuntu Bash version was not independently reproduced here, so this is not a claim that every CI runner currently fails this case.

Expected change: decide and encode the supported shell/truncation contract. Keep “no premature execution/no output leakage” distinct from “caller sees failure.” If nonzero status is promised, address the execution/download boundary rather than only weakening the assertion. Diagnostics should be tested by meaning rather than one English phrase.

Acceptance: preserve the historical cuts after bare set, after main, and after the npm installation line; preserve evidence that fake npm/REA commands did not execute. Validate on the supported Bash versions. Reducing the 7,537-process sweep is a performance option only if an equivalent structural/parser oracle and representative boundary coverage are demonstrated.

5. The cached local full gate omits real test inputs and does not observe changed host dependencies

test:run in turbo.json is cacheable and declares src/**, tests/**, scripts/**, bridge/**, toolchain/package files and selected generated outputs. It omits the root READMEs even though the existing consistency test reads them. check:test and check:pr use this task. The installed Turbo 2.11.7 documentation specifies deterministic known inputs as a precondition for reuse.

Existing-test reproduction: no synthetic test or runtime change is needed. Run the real README consistency test through Turbo with the original README, make the mutation from section 1, and compare cached and fresh execution:

node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \
  run test:run --output-logs=full -- tests/conformance/readmeConsistency.test.ts
# Make the README_zh.md mutation from section 1, then repeat the same command.
node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \
  run test:run --output-logs=full -- tests/conformance/readmeConsistency.test.ts
node node_modules/vitest/vitest.mjs run tests/conformance/readmeConsistency.test.ts
node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \
  run test:run --force -- tests/conformance/readmeConsistency.test.ts

The original run passes. After changing only README_zh.md, direct Vitest fails, both repeated Turbo runs return a cached pass with unchanged test:run hash 6d6c141eaa937a85, and --force fails with the same README assertion. Restoring the README makes direct Vitest pass. This is a confirmed missing repository input for an existing test, separate from the CI selection omission in section 1. All prerequisite tasks retained their hashes too; a generated-output change did not accidentally hide the omission.

External-dependency reproduction: external tool contents/host state are also not represented in these inputs. I tested this without changing the runtime version or the repository between the positive and negative runs:

Add a temporary process-boundary spec that invokes ci-review-provider on PATH and asserts that its JSON has healthy: true.
Put that executable outside the checkout; initially it returns {"healthy":true}.
Run the actual Turbo test:run task, passing the exact spec path. It passes and is cached.
Change only the external executable to return {"healthy":false}. Keep Node 24.18.0, npm 11.16.0, PATH and repository files identical.
Run Vitest directly, rerun Turbo twice, then rerun Turbo with --force.

Observation	Result
Initial Turbo run with healthy dependency	Pass
Direct Vitest with faulty dependency	Fail: `expected false to be true`
Turbo rerun with faulty dependency	Pass from cache, `test:run` hash `8cf172d08597a5e5`
Second Turbo rerun with faulty dependency	Same cached pass
Turbo `--force` with faulty dependency	Fail with the same assertion
Restore healthy dependency, direct Vitest	Pass

Task invocation:

node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \
  run test:run --output-logs=full -- tests/boundary/process/ciReviewExternalProvider.test.ts
# Fresh comparison:
node node_modules/vitest/vitest.mjs run tests/boundary/process/ciReviewExternalProvider.test.ts
# Force comparison:
node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \
  run test:run --force -- tests/boundary/process/ciReviewExternalProvider.test.ts

Temporary spec:

import { execFileSync } from 'node:child_process';
import { expect, it } from 'vitest';
it('checks the current external provider', () => {
  const observed = JSON.parse(execFileSync('ci-review-provider', { encoding: 'utf8' }));
  expect(observed.healthy).toBe(true);
});

Scope: the README case uses a real current-main test and an actual repository edit. The external-tool case verifies an additional cache-validity mechanism using a simulated dependency; it is not an observed LLDB/Ghidra/Hopper incident. The raw CI shard command invokes Vitest directly and does not reuse this local test-result cache. test:local and test:focused also run Vitest directly after any prerequisite preparation. Cached compilation and pure static checks remain useful.

An additional Node-24-to-Node-22 probe reproduced a stale cached pass, but changing away from the pinned developer toolchain weakens that example; the same-runtime external-dependency probe above is the primary evidence.

Expected change: account for the actual repository inputs read by cached tests, starting with README consistency, or execute the relevant checks under their owning uncached/fresh pipeline. Keep artifact preparation cacheable while ensuring host-dependent validation is executed, or explicitly distinguish a cached source-state check from a fresh host check. Do not snapshot/hash the entire ambient environment or expose its values. Merely hashing PATH would not fix the external-dependency case because PATH remained unchanged.

Acceptance: after caching success, changing the actual README input must invalidate the consistency check and fail. Faulting the same external dependency must cause fresh host validation to fail; restoring either control must pass. Unchanged deterministic build tasks should remain reusable.

6. Existing fast Python producer checks are not invoked by CI

bridge/native/test_rea_lldb_tracer.py loads the real producer module after installing a small fake lldb API. Its nine tests cover bounded journal retention, trace budgets, oversized serialization, launch flags, output-pipe behavior and breakpoint reporting.

python3 -m unittest bridge/native/test_rea_lldb_tracer.py -v

All nine passed under Python 3.12 in 0.395s. As a negative control in a temporary checkout, removing only the producer helper's eLaunchFlagStopAtEntry contribution caused the existing launch-flags test to fail with 10 != 14; restoring the source made all nine pass again. This confirms that the suite can detect a concrete producer regression.

The workflow/script graph has no invocation of this suite. Vitest's discovery guard verifies the TypeScript/Vitest inventory, so its success does not include this file. Related TypeScript transport tests and real Apple artifact checks do not establish that these nine producer assertions execute in CI.

Expected change / acceptance: give this existing suite an explicit inexpensive CI owner and include its status in the relevant gate; demonstrate that a producer regression causes it to fail. It does not require a real LLDB installation, and running it must not be described as full real-engine verification. This is an automation coverage gap, not proof that the producer implementation is currently wrong.

Measured performance opportunities

These observations explain where to investigate next. They are not claims that all affected checks or cases are redundant.

A. Broad selection and queue time can dominate PR feedback

Nine successful PR runs were inspected, including seven full selections and two narrower selections. Across all nine, workflow wall time ranged from 8.82 to 45.33 minutes, median 15.10 minutes. The seven full selections each launched 31 jobs and consumed approximately 68–76 summed job-execution minutes. This is execution time, not GitHub billing or a quota estimate.

In run 38237724705, Linux shards finished around 11.7 minutes after workflow creation, the Apple job started around 29.2 minutes and ended around 36.4 minutes, and the final required job started around 45.1 minutes. Reducing test body time alone cannot explain or recover all of that delay. The timestamps demonstrate scheduling delay; the exact runner/account constraint was not established.

Replaying 120 first-parent changes as hypothetical PR diffs against the current planner selected the full set for 94/120 (78.3%). This is a planner replay, not the actual historical PR rate. Both explicit shared boundaries and unclassified sources contribute. For example, src/application/BinaryTargetResolver.ts triggers an unclassified-input full fallback on main; #2131 changes it to a Linux runtime fallback.

Follow-up: evaluate #2131 rather than duplicate it. Measure wall time, queue delay, runner-minutes, memory and the guarantees removed by narrower ownership. Six shards may shorten execution but can also increase startup work and queue contention. Keep an observable successful full-main baseline: in one busy sample of 40 main runs, 24 were cancelled, so “main covers it later” needs actual evidence.

B. The outer build-cache key prevents partial reuse across otherwise reusable task inputs

The portable-build-cache action aggregates all six eager task hashes into one cache key and supplies no restore-keys.

A controlled comment-only change to scripts/fixtures/managed/function-dossier.mjs changed only artifacts:managed-evidence's task hash. The other five hashes, including compile, were unchanged, but the outer key changed. Restoring the original bytes restored every hash and the aggregate key. This demonstrates avoidable loss of access to reusable task entries on an outer cache miss; Turbo's task-level hashing itself worked correctly.

Static/docs jobs do not restore that portable task cache on main. Apple/readiness/other verifier entrypoints can independently compile. Jobs that begin before the build job publishes a new cross-run cache can also miss it. Typical observed duplicate compile steps cost about 9–15s; full job medians were approximately 47s for static, 32s for docs, 354s for Apple, 385s for Windows, and 426s for macOS Intel packaging. Build reuse helps, but it is not a credible explanation for the whole 45-minute tail.

Follow-up: permit carefully scoped access to prior task entries and let the installed Turbo version validate task inputs, or reuse immutable per-run build artifacts where latency permits. Respect OS/architecture/toolchain and GitHub's cache scope/matching rules. Avoid adding a serial build dependency to every verifier without measuring the critical path. #2131 already adds Apple/Windows cache reuse and removes a Windows typecheck; those parts should be reviewed there.

Acceptance: an owner-specific input change invalidates its task while reusing unchanged compilation; actual compilation inputs still invalidate correctly; native artifacts are never reused across incompatible targets.

C. Execution hotspots matter more than blanket test deletion

The successful run 38239959095 report contains 915 test files, 9,376 cases, 9,290 passed and 86 pending. Its summed file durations are about 2,163s; the slowest 50 files account for about 1,097s (50.7%). These are summed execution durations, not workflow wall time.

Representative slow files include applicationWorkflowCli.test.ts (~55.7s), clientProfiles.test.ts (~53.6s), exportedFunctionBindingsCli.test.ts (~52.0s), processFinalization.test.ts (~42.3s), and localToolContracts.test.ts (~41.9s). Investigate repeated CLI/MCP setup, analysis and child-process startup by test family. For example, the 64 exported-function cases already share MCP setup and include a real Node loader oracle, so deleting them merely because they are slow would discard identified semantics.

The initial Linux full suite without coverage took 26m34.77s, with maxWorkers=2: 901 passed / 13 skipped / 1 failed files, and 9,266 passed / 89 skipped / 1 failed cases. The one failure was the installer prefix case above. The host was shared with a few investigation probes, so this is a diagnostic run rather than an isolated benchmark.

A two-file coverage comparison took 11.51s without coverage and 30.35s with coverage. It ran alongside the full suite and is too small/contended to extrapolate a whole-suite 2.64× improvement. An eight-invocation bytecode-cache probe showed only about 3% warm improvement. Neither result justifies a global promise or coverage reduction.

Follow-up: optimize a small number of the slowest families while preserving distinct malformed-input, cleanup, cancellation, capacity and identity oracles. Compare cold/warm and coverage/non-coverage runs on comparable runners before changing shard count or coverage policy.

D. Timing transport and static logs can be made smaller

The build job downloads the previous successful run's full test-results artifact to obtain one timing JSON. A sampled artifact was ~7.5MB compressed, including 1,315 coverage HTML files (~34.64MiB unpacked), while a normalized timing map would be ~55KB. A dedicated timing artifact is a small, concrete transport/storage improvement; it is not the dominant test bottleneck.

One static log contained 857 lint warnings and 1,219 printed clone blocks. The clone report covered 2,142 files with TypeScript duplication at 2.77%, below the 3% threshold. This is a feedback/noise problem, not evidence that lint, typecheck, discovery, formatting, or duplication checking are all unnecessary.

Normal local development is already materially lighter: warm check:fast took about 0.5s, and cold/fresh runs about 6–8.5s. The complete check:pr gate is documented as optional. Preserve that distinction in developer guidance.

Findings rejected or narrowed during review

No evidence of widespread tests importing nonexistent production code. The literal relative-import scan found zero unresolved paths, and all 915 tracked Vitest specs were discovered exactly once. This does not prove every assertion has the right semantic contract, but it rejects the blanket “the suite mostly tests removed code” diagnosis.
Tests without direct production imports are not automatically dead: many execute the CLI, MCP, subprocesses, Python bridges or installed packages.
The historical readiness/prompt-completion drift is already fixed. Closed #2123 records a genuine old fixture/contract mismatch. It should not be reported as an active current-main failure.
The history-aware sharder is not demonstrably broken. It assigned all 915 known files with historical sums approximately 504.6s per shard. Observed runtimes were uneven, but changing environments and project serialization explain uncertainty; imbalance alone does not prove an algorithm defect.
Broad shared-boundary selection is a conservative coverage policy. Narrowing it is a documented tradeoff requiring owner/consumer evidence, rather than proof the existing checks serve no purpose.
Failure/cancellation/action-required counts are not a flake rate. Missing external real-engine execution is unknown coverage, not a passing simulated-provider result.
Queue delay, an approximately 10GiB cache snapshot, and small coverage/bytecode probes do not establish account quotas, cache-eviction causality, or a guaranteed optimization multiplier.

Proposed order and completion criteria

Close the README-only guard gap with the existing small consistency test.
Complete/review native test selection through #2131, with host-owner regressions.
Preserve merged failed-shard diagnostics while keeping required status and aggregate coverage enforcement strict.
Resolve the installer shell contract and portable oracle before reducing prefix coverage.
Correct cached test-input ownership and make fresh host validation explicit; prove both an actual README edit and an external-dependency fault cannot be hidden by a prior pass.
Give the nine existing LLDB Python producer checks a cheap automated owner.
Measure the combined effect of #2131, cache reuse and queue changes on comparable runs; record any removed platform/provider guarantee.
Then optimize the slowest test families and the timing/log payloads with retained semantic coverage.

Use the reproductions and retained coverage contracts above as the acceptance criteria for follow-up changes.

Originally created by @N0zoM1z0 on GitHub (Oct 12, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2190 Originally assigned to: @N0zoM1z0 on GitHub. ## Implementation progress (2026-10-12) The initial audit below is retained as historical evidence. The verified selection/cache/reporting/installer/README/Python gaps are now addressed by merged #2131 and #2210. Follow-up #2238 was assigned to N0zoM1z0 and closed by merged #2242 (`d96550f2`): concise static feedback with complete diagnostic artifacts, unchanged lint/duplicate failure gates, portable unambiguous clone locations, and cached lint ownership for all observed source roots. The final PR passed all 33 checks: 946 files / 9,748 cases, 9,662 passed, 86 skipped, zero failed. Its actual static log has 398 lines / 29,093 bytes, and the complete artifact retains 911 lint diagnostics and 1,292 clones. A real website syntax error previously passed cached check:fast but failed fresh lint; the corrected cache now rejects it. The independent live checker-inventory/Turbo-input regression fails on the old ownership. Slow-family and queue follow-ups were investigated rather than assumed: three complete 99-case runs all passed but showed substantial timing variation; the explicit live-definition MCP prototype improved cold-request cost but did not establish stable full-family speedup. It was restored and no additional test refactor is merged. Three Actions samples show queue medians from 4 to 49 seconds and maxima from 75 to 199 seconds; these do not prove a runner/account capacity cause or justify reducing platform/shard coverage. Those uncertain performance items remain open; no CI-wide speed percentage is claimed. Current work stops after this evidence-backed merge. ## Problem and scope The CI/test audit found several reproducible verification gaps and a failure-reporting problem. It also found substantial CI cost, but the evidence does **not** support deleting large parts of the test suite or treating every broad lane selection as a bug. This issue records the reviewed findings, reproduction steps, measured costs, and acceptance criteria. Correctness findings and performance opportunities are separated below so that optimization does not silently remove coverage. **Baseline:** `main` at [`5190843f8f09ac17cbf79561bd1f708c509cda78`](https://github.com/morluto/rea/commit/5190843f8f09ac17cbf79561bd1f708c509cda78), verified against the remote on 2026-10-12. Tests used the repository-pinned Node **24.18.0**, npm **11.16.0**, and Python **3.12** for the website workflow. The installer reproduction used GNU Bash **5.2.15** on Linux. The initial full Linux test run was on `672bdea21`; the two subsequent main commits changed managed CLI schema handling and uninstall output, with no changes to CI, the planner, Turbo, Vitest, or the installer. Both changed test files were rerun on `5190843f8` and all 11 cases passed. The fault-injection and selection probes below were performed against `5190843f8` in isolated temporary checkouts. Project source and workflow files have not been changed as part of this investigation. Related work: [#2131](https://github.com/morluto/rea/pull/2131), `ci: narrow native-host lane selection and reuse build caches`, is open/unmerged at `d2e75e22115621cc0a2a89aa24904635fc93134f`. Its actual planner was tested as a comparison. Findings already addressed by that PR are identified explicitly; this issue should not create a competing implementation. ## Reviewed findings | Finding | Evidence and status | Suggested priority | | --- | --- | --- | | README-only changes bypass an existing command-parity test | Real README mutation: the existing test fails twice, while the selected docs and website checks pass | P1 | | Native test-only changes do not select their host lane | Actual main planner reproduces the omission; the planner in #2131 selects the missing host lane | P1; track through #2131 | | Failed shards prevent merging otherwise usable diagnostics | Observed in Actions; controlled failing shard still produces a mergeable blob, JSON and JUnit | P1 | | Installer prefix test assumes Bash status/diagnostic behavior that is not universal | Existing test fails again on current main; a minimal Bash program independently reproduces status 0 without executing the guarded body | P1 | | Cached local full gates omit a real README input and can hide changed host dependencies | Existing README test rejects an actual repository edit, but the cached task passes twice; independent external-dependency injection gives the same result | P1 for missing repository inputs; P2 for host-validation policy | | Existing LLDB producer unit tests are outside the automated CI inventory | Nine existing Python tests pass in 0.395s; no invocation in the current workflow/script graph | P2; coverage gap, not a demonstrated producer defect | ### 1. README-only PRs bypass the existing onboarding command-parity guard The [README classification branch](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/scripts/ci/plan.mjs#L120-L132) selects `docs` and, for the relevant README paths, `website`, then continues without selecting `code`. The existing [readmeConsistency.test.ts](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/tests/conformance/readmeConsistency.test.ts#L37-L59) therefore does not run on these PRs. That test compares translated shell examples with the English examples and checks the installation MCP example. This is an actual routing gap rather than an assumption that docs checks are weak. In an isolated checkout, I replaced the first fenced Bash example in `README_zh.md` with `rea-command-that-does-not-exist --json`: | Check | Original README | Mutated README | Restored README | | --- | --- | --- | --- | | `npm run test:local -- tests/conformance/readmeConsistency.test.ts` | Pass, 2 tests | Fail twice, explicitly naming `README_zh.md` | Pass | | `npm run docs:check` | — | Pass; the first run executed all six tasks rather than relying on a cache hit | — | | Python 3.12 `scripts/website.py check` | — | Pass, 97.08s | — | | Planner for a README-only diff | — | `code=false`, `docs=true`, `website=true` | — | Reproduce in a disposable checkout with locked dependencies installed: ```sh npm run test:local -- tests/conformance/readmeConsistency.test.ts python3 - <<'PY' from pathlib import Path import re p = Path('README_zh.md') p.write_text(re.sub(r'```bash\s*([\s\S]*?)```', '```bash\nrea-command-that-does-not-exist --json\n```', p.read_text(), count=1)) PY npm run test:local -- tests/conformance/readmeConsistency.test.ts npm run docs:check # Use Python 3.12 with website/requirements.txt installed: python3 scripts/website.py check ``` The second test command fails, while both content pipelines accept the mutation. The oracle here is **command parity**; the existing test is not a complete semantic validator of every CLI example. **Expected change:** run this small existing guard in the docs lane, or move its relevant contract into the docs validation owner. Selecting the entire Linux suite for a translated README edit is unnecessary to close this particular gap. **Acceptance:** the mutation above fails a README-only PR; the restored README passes; unrelated translated prose does not trigger the complete runtime/provider matrix. If #2141 moves translated READMEs, update both ownership and this guard together. ### 2. Native test-only edits can produce a green PR without exercising the edited host-specific case The main planner's [generic test-file early return](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/scripts/ci/plan.mjs#L129-L132) happens before native ownership classification. Two concrete existing paths demonstrate the problem: | Only changed path | Main planner | Relevant host behavior | Planner from #2131 | | --- | --- | --- | --- | | `tests/boundary/filesystem/windowsNpxRegistration.test.ts` | `code=true`, `package=false`, `apple=false` | Windows-only registration cases skip on Linux; the Windows curated job requires `package=true` | `package=true`, `runtime=true` | | `tests/boundary/cli/nativePlistCli.test.ts` | `code=true`, `apple=false`, `package=false` | Its suites skip outside Darwin; the Apple job requires `apple=true` | `apple=true` | Sources: [Windows platform condition](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/tests/boundary/filesystem/windowsNpxRegistration.test.ts#L148), [Darwin platform condition](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/tests/boundary/cli/nativePlistCli.test.ts#L12), [Windows job and curated test](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/.github/workflows/ci.yml#L446-L489), [Apple job and test](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/.github/workflows/ci.yml#L373-L402). I exercised the actual planner against small temporary Git repositories containing a base commit and exactly one changed path. Each classification was repeated and compared for equality. Controls included an ordinary domain test, a README-only change, an unowned runtime source, and `CI_FULL=true`. Ordinary tests still selected the Linux code lane; the full override selected every lane. Equivalent reproduction in a disposable checkout: ```sh REA_REVIEW_BASE=$(git rev-parse HEAD) printf '\n// selection probe\n' >> tests/boundary/cli/nativePlistCli.test.ts git -c user.name='CI review' -c user.email='ci-review@example.invalid' \ commit -am 'test: probe native test selection' GITHUB_EVENT_NAME=pull_request BASE_SHA="$REA_REVIEW_BASE" \ HEAD_SHA=HEAD CI_FULL=false node scripts/ci/plan.mjs ``` Repeat with the Windows path. These are test-selection reproductions; no macOS/Windows behavior is claimed to have been executed on Linux. **Expected change / acceptance:** use the owner-aware fix in #2131; retain deterministic planner regressions asserting that a changed platform-specific test selects its executing host. Preserve ordinary source-test feedback and the explicit full-baseline override. Main pushes already select a full baseline, but that does not repair missing PR feedback. ### 3. A failing shard blocks report merging before diagnosis can be collected The aggregate [test job](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/.github/workflows/ci.yml#L200-L270) first requires all shards to succeed. Checkout, dependency installation, blob download, and merging then require `success()`. When a shard fails, those steps are skipped. The final `always()` upload runs without the merged files and fails as well. This happened in [run 38224038323, aggregate job 114730238104](https://github.com/morluto/rea/actions/runs/38224038323/job/114730238104), and also in [#2131's run 38234806511, aggregate job 114764631340](https://github.com/morluto/rea/actions/runs/38234806511/job/114764631340): ```text Require successful test shards failure Checkout / setup / npm ci / blob download skipped Merge coverage, JUnit, and timing reports skipped Upload test-results failure ``` This follows GitHub's documented [status-check semantics](https://docs.github.com/en/actions/reference/workflows-and-actions/expressions#status-check-functions). Individual shard logs/blobs remain available; the loss is the merged report and centralized diagnosis, not all evidence of the failure. To verify that merging failed-test blobs is actually useful, I added one passing and one deliberately failing temporary Vitest spec, then ran the real shard and merge scripts: ```sh npm run test:ci:shard:run -- \ src/domain/ciReviewControl.test.ts src/domain/ciReviewFailure.test.ts --shard=1/2 npm run test:ci:shard:run -- \ src/domain/ciReviewControl.test.ts src/domain/ciReviewFailure.test.ts --shard=2/2 npm run test:ci:merge -- \ --coverage.reportsDirectory=coverage \ --coverage.reporter=json-summary \ --reporter=json --reporter=junit \ --outputFile.json=test-results.json --outputFile.junit=test-results.xml ``` The temporary specs respectively used `it('control', () => expect(true).toBe(true))` and `it('controlled failure', () => { throw new Error('controlled-shard-failure'); })`, with imports from Vitest. Observed results: - Shard 1 exited 0; shard 2 exited 1. **Both wrote blobs.** - Merge exited 1 and wrote valid JSON/JUnit reporting **two cases: one passed, one failed**. - The tiny fixture also failed normal aggregate coverage floors, as expected; this was not a coverage-performance benchmark. - The actual `scripts/ci/required.mjs` rejected a failed-shard job set and accepted the all-success control. **Expected change:** collect available non-cancelled shard diagnostics before the final success gate. Keep failure/missing-shard/coverage-floor enforcement in the result contract. A report from incomplete shards must be marked incomplete and must never become the successful timing-history baseline. **Acceptance:** a controlled failing shard still yields a merged test-results artifact and a failing required check. Missing blobs, cancelled jobs, and failure before Vitest starts are handled explicitly. Do not simply add `continue-on-error` and make the gate green. This is distinct from the historical coverage-temp ENOENT in closed #150. ### 4. The exhaustive installer prefix test has an incorrect Bash oracle on this host The [prefix test](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/tests/boundary/filesystem/installScript.test.ts#L217-L276) starts one Bash process per byte prefix of `install.sh`: **7,537 subprocesses** for the current 7,536-byte file, in batches of 16. It requires every incomplete body prefix to exit nonzero and match `syntax error|unexpected EOF`. On Bash 5.2.15, a malformed/truncated `[[ ... ]]` can report a parsing error while returning status 0. Other prefixes report `unexpected token 'EOF' in conditional command`, which is also outside the test's regex. A minimal independent reproduction is: ```sh python3 - <<'PY' import subprocess p = subprocess.run(['/bin/bash', '-s'], input='{\nprintf "EXECUTED\\n"\nif [[ "$verbose" == true ]', text=True, capture_output=True) print('status:', p.returncode) print('stdout:', repr(p.stdout)) print('stderr:', repr(p.stderr)) PY ``` Observed: `status: 0`, empty stdout, and `syntax error in conditional expression`. The sentinel in the brace group does **not** execute. This result was repeated under `C` and `C.UTF-8`. Positive controls showed a balanced group executes the sentinel, an ordinary incomplete group exits nonzero, and removing the guard lets the sentinel execute before the parse error. The existing production-boundary test was rerun through the supported command: ```sh npm run test:focused -- tests/boundary/filesystem/installScript.test.ts ``` Current-main result: **29 cases passed, one failed**, 26.40s including command setup / 25.35s Vitest. The same prefix case failed during the initial full suite, taking 30.23s. Recorded failures concerned exit status and diagnostic wording, rather than observed early stdout execution. **Interpretation:** the shell test is genuinely nonportable on this host. Separately, a truncated installer pipeline returning status 0 can mislead a caller even if it makes no changes. The independent reproduction does not prove that npm ran or that the brace guard leaked environment data. CI's current Ubuntu Bash version was not independently reproduced here, so this is not a claim that every CI runner currently fails this case. **Expected change:** decide and encode the supported shell/truncation contract. Keep “no premature execution/no output leakage” distinct from “caller sees failure.” If nonzero status is promised, address the execution/download boundary rather than only weakening the assertion. Diagnostics should be tested by meaning rather than one English phrase. **Acceptance:** preserve the historical cuts after bare `set`, after `main`, and after the npm installation line; preserve evidence that fake npm/REA commands did not execute. Validate on the supported Bash versions. Reducing the 7,537-process sweep is a performance option only if an equivalent structural/parser oracle and representative boundary coverage are demonstrated. ### 5. The cached local full gate omits real test inputs and does not observe changed host dependencies [`test:run` in turbo.json](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/turbo.json#L106-L132) is cacheable and declares `src/**`, `tests/**`, `scripts/**`, `bridge/**`, toolchain/package files and selected generated outputs. It omits the root READMEs even though the existing consistency test reads them. [check:test and check:pr](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/package.json#L111-L113) use this task. The installed Turbo 2.11.7 documentation specifies deterministic known inputs as a precondition for reuse. **Existing-test reproduction:** no synthetic test or runtime change is needed. Run the real README consistency test through Turbo with the original README, make the mutation from section 1, and compare cached and fresh execution: ```sh node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \ run test:run --output-logs=full -- tests/conformance/readmeConsistency.test.ts # Make the README_zh.md mutation from section 1, then repeat the same command. node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \ run test:run --output-logs=full -- tests/conformance/readmeConsistency.test.ts node node_modules/vitest/vitest.mjs run tests/conformance/readmeConsistency.test.ts node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \ run test:run --force -- tests/conformance/readmeConsistency.test.ts ``` The original run passes. After changing only `README_zh.md`, direct Vitest fails, both repeated Turbo runs return a cached pass with unchanged `test:run` hash `6d6c141eaa937a85`, and `--force` fails with the same README assertion. Restoring the README makes direct Vitest pass. This is a **confirmed missing repository input for an existing test**, separate from the CI selection omission in section 1. All prerequisite tasks retained their hashes too; a generated-output change did not accidentally hide the omission. **External-dependency reproduction:** external tool contents/host state are also not represented in these inputs. I tested this without changing the runtime version or the repository between the positive and negative runs: 1. Add a temporary process-boundary spec that invokes `ci-review-provider` on PATH and asserts that its JSON has `healthy: true`. 2. Put that executable outside the checkout; initially it returns `{"healthy":true}`. 3. Run the actual Turbo `test:run` task, passing the exact spec path. It passes and is cached. 4. Change only the external executable to return `{"healthy":false}`. Keep Node 24.18.0, npm 11.16.0, PATH and repository files identical. 5. Run Vitest directly, rerun Turbo twice, then rerun Turbo with `--force`. | Observation | Result | | --- | --- | | Initial Turbo run with healthy dependency | Pass | | Direct Vitest with faulty dependency | Fail: `expected false to be true` | | Turbo rerun with faulty dependency | Pass from cache, `test:run` hash `8cf172d08597a5e5` | | Second Turbo rerun with faulty dependency | Same cached pass | | Turbo `--force` with faulty dependency | Fail with the same assertion | | Restore healthy dependency, direct Vitest | Pass | Task invocation: ```sh node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \ run test:run --output-logs=full -- tests/boundary/process/ciReviewExternalProvider.test.ts # Fresh comparison: node node_modules/vitest/vitest.mjs run tests/boundary/process/ciReviewExternalProvider.test.ts # Force comparison: node scripts/run-exclusive.mjs artifacts node node_modules/turbo/bin/turbo \ run test:run --force -- tests/boundary/process/ciReviewExternalProvider.test.ts ``` Temporary spec: ```ts import { execFileSync } from 'node:child_process'; import { expect, it } from 'vitest'; it('checks the current external provider', () => { const observed = JSON.parse(execFileSync('ci-review-provider', { encoding: 'utf8' })); expect(observed.healthy).toBe(true); }); ``` **Scope:** the README case uses a real current-main test and an actual repository edit. The external-tool case verifies an additional cache-validity mechanism using a simulated dependency; it is not an observed LLDB/Ghidra/Hopper incident. The raw CI shard command invokes Vitest directly and does not reuse this local test-result cache. `test:local` and `test:focused` also run Vitest directly after any prerequisite preparation. Cached compilation and pure static checks remain useful. An additional Node-24-to-Node-22 probe reproduced a stale cached pass, but changing away from the pinned developer toolchain weakens that example; the same-runtime external-dependency probe above is the primary evidence. **Expected change:** account for the actual repository inputs read by cached tests, starting with README consistency, or execute the relevant checks under their owning uncached/fresh pipeline. Keep artifact preparation cacheable while ensuring host-dependent validation is executed, or explicitly distinguish a cached source-state check from a fresh host check. Do not snapshot/hash the entire ambient environment or expose its values. Merely hashing PATH would not fix the external-dependency case because PATH remained unchanged. **Acceptance:** after caching success, changing the actual README input must invalidate the consistency check and fail. Faulting the same external dependency must cause fresh host validation to fail; restoring either control must pass. Unchanged deterministic build tasks should remain reusable. ### 6. Existing fast Python producer checks are not invoked by CI [`bridge/native/test_rea_lldb_tracer.py`](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/bridge/native/test_rea_lldb_tracer.py#L1-L24) loads the real producer module after installing a small fake `lldb` API. Its nine tests cover bounded journal retention, trace budgets, oversized serialization, launch flags, output-pipe behavior and breakpoint reporting. ```sh python3 -m unittest bridge/native/test_rea_lldb_tracer.py -v ``` All nine passed under Python 3.12 in **0.395s**. As a negative control in a temporary checkout, removing only the producer helper's `eLaunchFlagStopAtEntry` contribution caused the existing launch-flags test to fail with `10 != 14`; restoring the source made all nine pass again. This confirms that the suite can detect a concrete producer regression. The workflow/script graph has no invocation of this suite. Vitest's discovery guard verifies the TypeScript/Vitest inventory, so its success does not include this file. Related TypeScript transport tests and real Apple artifact checks do not establish that these nine producer assertions execute in CI. **Expected change / acceptance:** give this existing suite an explicit inexpensive CI owner and include its status in the relevant gate; demonstrate that a producer regression causes it to fail. It does not require a real LLDB installation, and running it must not be described as full real-engine verification. This is an automation coverage gap, not proof that the producer implementation is currently wrong. ## Measured performance opportunities These observations explain where to investigate next. They are not claims that all affected checks or cases are redundant. ### A. Broad selection and queue time can dominate PR feedback Nine successful PR runs were inspected, including seven full selections and two narrower selections. Across all nine, workflow wall time ranged from **8.82 to 45.33 minutes**, median **15.10 minutes**. The seven full selections each launched **31 jobs** and consumed approximately **68–76 summed job-execution minutes**. This is execution time, not GitHub billing or a quota estimate. In [run 38237724705](https://github.com/morluto/rea/actions/runs/38237724705), Linux shards finished around 11.7 minutes after workflow creation, the Apple job started around 29.2 minutes and ended around 36.4 minutes, and the final required job started around 45.1 minutes. Reducing test body time alone cannot explain or recover all of that delay. The timestamps demonstrate scheduling delay; the exact runner/account constraint was not established. Replaying **120 first-parent changes as hypothetical PR diffs** against the current planner selected the full set for **94/120 (78.3%)**. This is a planner replay, not the actual historical PR rate. Both explicit shared boundaries and unclassified sources contribute. For example, `src/application/BinaryTargetResolver.ts` triggers an unclassified-input full fallback on main; #2131 changes it to a Linux runtime fallback. **Follow-up:** evaluate #2131 rather than duplicate it. Measure wall time, queue delay, runner-minutes, memory and the guarantees removed by narrower ownership. Six shards may shorten execution but can also increase startup work and queue contention. Keep an observable successful full-main baseline: in one busy sample of 40 main runs, 24 were cancelled, so “main covers it later” needs actual evidence. ### B. The outer build-cache key prevents partial reuse across otherwise reusable task inputs The [portable-build-cache action](https://github.com/morluto/rea/blob/5190843f8f09ac17cbf79561bd1f708c509cda78/.github/actions/portable-build-cache/action.yml#L10-L40) aggregates all six eager task hashes into one cache key and supplies no `restore-keys`. A controlled comment-only change to `scripts/fixtures/managed/function-dossier.mjs` changed **only** `artifacts:managed-evidence`'s task hash. The other five hashes, including `compile`, were unchanged, but the outer key changed. Restoring the original bytes restored every hash and the aggregate key. This demonstrates avoidable loss of access to reusable task entries on an outer cache miss; Turbo's task-level hashing itself worked correctly. Static/docs jobs do not restore that portable task cache on main. Apple/readiness/other verifier entrypoints can independently compile. Jobs that begin before the build job publishes a new cross-run cache can also miss it. Typical observed duplicate compile steps cost about **9–15s**; full job medians were approximately 47s for static, 32s for docs, 354s for Apple, 385s for Windows, and 426s for macOS Intel packaging. Build reuse helps, but it is not a credible explanation for the whole 45-minute tail. **Follow-up:** permit carefully scoped access to prior task entries and let the installed Turbo version validate task inputs, or reuse immutable per-run build artifacts where latency permits. Respect OS/architecture/toolchain and GitHub's [cache scope/matching rules](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching). Avoid adding a serial build dependency to every verifier without measuring the critical path. #2131 already adds Apple/Windows cache reuse and removes a Windows typecheck; those parts should be reviewed there. **Acceptance:** an owner-specific input change invalidates its task while reusing unchanged compilation; actual compilation inputs still invalidate correctly; native artifacts are never reused across incompatible targets. ### C. Execution hotspots matter more than blanket test deletion The successful [run 38239959095](https://github.com/morluto/rea/actions/runs/38239959095) report contains **915 test files, 9,376 cases, 9,290 passed and 86 pending**. Its summed file durations are about **2,163s**; the slowest **50 files account for about 1,097s (50.7%)**. These are summed execution durations, not workflow wall time. Representative slow files include `applicationWorkflowCli.test.ts` (~55.7s), `clientProfiles.test.ts` (~53.6s), `exportedFunctionBindingsCli.test.ts` (~52.0s), `processFinalization.test.ts` (~42.3s), and `localToolContracts.test.ts` (~41.9s). Investigate repeated CLI/MCP setup, analysis and child-process startup by test family. For example, the 64 exported-function cases already share MCP setup and include a real Node loader oracle, so deleting them merely because they are slow would discard identified semantics. The initial Linux full suite without coverage took **26m34.77s**, with maxWorkers=2: **901 passed / 13 skipped / 1 failed files**, and **9,266 passed / 89 skipped / 1 failed cases**. The one failure was the installer prefix case above. The host was shared with a few investigation probes, so this is a diagnostic run rather than an isolated benchmark. A two-file coverage comparison took 11.51s without coverage and 30.35s with coverage. It ran alongside the full suite and is too small/contended to extrapolate a whole-suite 2.64× improvement. An eight-invocation bytecode-cache probe showed only about 3% warm improvement. Neither result justifies a global promise or coverage reduction. **Follow-up:** optimize a small number of the slowest families while preserving distinct malformed-input, cleanup, cancellation, capacity and identity oracles. Compare cold/warm and coverage/non-coverage runs on comparable runners before changing shard count or coverage policy. ### D. Timing transport and static logs can be made smaller The build job downloads the previous successful run's full `test-results` artifact to obtain one timing JSON. A sampled artifact was ~7.5MB compressed, including 1,315 coverage HTML files (~34.64MiB unpacked), while a normalized timing map would be ~55KB. A dedicated timing artifact is a small, concrete transport/storage improvement; it is not the dominant test bottleneck. One static log contained 857 lint warnings and 1,219 printed clone blocks. The clone report covered 2,142 files with TypeScript duplication at 2.77%, below the 3% threshold. This is a feedback/noise problem, not evidence that lint, typecheck, discovery, formatting, or duplication checking are all unnecessary. Normal local development is already materially lighter: warm `check:fast` took about 0.5s, and cold/fresh runs about 6–8.5s. The complete `check:pr` gate is documented as optional. Preserve that distinction in developer guidance. ## Findings rejected or narrowed during review - **No evidence of widespread tests importing nonexistent production code.** The literal relative-import scan found zero unresolved paths, and all 915 tracked Vitest specs were discovered exactly once. This does not prove every assertion has the right semantic contract, but it rejects the blanket “the suite mostly tests removed code” diagnosis. - Tests without direct production imports are not automatically dead: many execute the CLI, MCP, subprocesses, Python bridges or installed packages. - **The historical readiness/prompt-completion drift is already fixed.** Closed [#2123](https://github.com/morluto/rea/issues/2123) records a genuine old fixture/contract mismatch. It should not be reported as an active current-main failure. - **The history-aware sharder is not demonstrably broken.** It assigned all 915 known files with historical sums approximately 504.6s per shard. Observed runtimes were uneven, but changing environments and project serialization explain uncertainty; imbalance alone does not prove an algorithm defect. - Broad shared-boundary selection is a conservative coverage policy. Narrowing it is a documented tradeoff requiring owner/consumer evidence, rather than proof the existing checks serve no purpose. - Failure/cancellation/action-required counts are not a flake rate. Missing external real-engine execution is unknown coverage, not a passing simulated-provider result. - Queue delay, an approximately 10GiB cache snapshot, and small coverage/bytecode probes do not establish account quotas, cache-eviction causality, or a guaranteed optimization multiplier. ## Proposed order and completion criteria - [x] Close the README-only guard gap with the existing small consistency test. - [x] Complete/review native test selection through #2131, with host-owner regressions. - [x] Preserve merged failed-shard diagnostics while keeping required status and aggregate coverage enforcement strict. - [x] Resolve the installer shell contract and portable oracle before reducing prefix coverage. - [x] Correct cached test-input ownership and make fresh host validation explicit; prove both an actual README edit and an external-dependency fault cannot be hidden by a prior pass. - [x] Give the nine existing LLDB Python producer checks a cheap automated owner. - [ ] Measure the combined effect of #2131, cache reuse and queue changes on comparable runs; record any removed platform/provider guarantee. - [ ] Then optimize the slowest test families and the timing/log payloads with retained semantic coverage. Use the reproductions and retained coverage contracts above as the acceptance criteria for follow-up changes.

bsakaguchi added the

labels

2026-10-15 18:27:46 +00:00

bsakaguchi commented

2026-10-15 18:27:47 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Performance follow-up, current main 1579b6865e8ee87873036a39af3e257cc70d6f21: two same-Git-tree hosted samples show verify:package:windows taking 222s versus 558s (runs 38585311071 and 38587680181). The latter step's build preparation was already cached (~106ms), so rebuilding is not an established explanation for the remaining delay.

A temporary manual-only diagnostic branch (15f44d640d4910051016851f194d4161a1e3a01c, not an integration PR) profiles the full real verifier: pack / install / CLI help / MCP catalog / setup and idempotence / uninstall / real npm update / cleanup. It builds the native Windows artifact from the exact same head and workflow run, retains all existing assertions and commands, and records phase durations plus aggregate upstream-registry traffic. Update-host operations delegate directly to the production host; no success or dependency output is cached.

A local pinned-npm-script control already refutes the simple assumption that changing HOME necessarily loses the npm download cache: npm exports its configured npm_config_cache to scripts, and the runtime only strips npm_command. Windows propagation and the dominant phase remain to be measured. The Windows-only verifier also does not perform the separate initial global-install step (that source branch runs on non-Windows hosts); the updater's real global replacement is retained. No root cause, optimization or speedup is claimed yet.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Performance follow-up, current main `1579b6865e8ee87873036a39af3e257cc70d6f21`: two same-Git-tree hosted samples show `verify:package:windows` taking 222s versus 558s (runs 38585311071 and 38587680181). The latter step's build preparation was already cached (~106ms), so rebuilding is not an established explanation for the remaining delay. A temporary manual-only diagnostic branch (`15f44d640d4910051016851f194d4161a1e3a01c`, not an integration PR) profiles the full real verifier: pack / install / CLI help / MCP catalog / setup and idempotence / uninstall / real npm update / cleanup. It builds the native Windows artifact from the exact same head and workflow run, retains all existing assertions and commands, and records phase durations plus aggregate upstream-registry traffic. Update-host operations delegate directly to the production host; no success or dependency output is cached. A local pinned-npm-script control already refutes the simple assumption that changing HOME necessarily loses the npm download cache: npm exports its configured `npm_config_cache` to scripts, and the runtime only strips `npm_command`. Windows propagation and the dominant phase remain to be measured. The Windows-only verifier also does **not** perform the separate initial global-install step (that source branch runs on non-Windows hosts); the updater's real global replacement is retained. No root cause, optimization or speedup is claimed yet.

bsakaguchi commented

2026-10-15 18:27:48 +00:00

Author

Owner

@N0zoM1z0 commented on GitHub (Oct 14, 2026):

Bounded Windows profile completed successfully: run 38602408204, exact diagnostic head 15f44d640d4910051016851f194d4161a1e3a01c, production baseline main 1579b6865. Both jobs passed; the native artifact was built from that same run/head. Artifact windows-package-phase-profile: ID 11915231329.

Actual verifier phase	Wall seconds
pack, including its normal prepack checks	5.92
fresh local installation	122.02
CLI help	1.22
MCP catalog and lifecycle	10.84
setup / idempotence	2.51 / 1.83
uninstall	1.32
update total	117.84
updater's real npm installation (inside update total)	109.46
cleanup	8.54
complete traced interval	272.24

All original package/native-file, complete MCP catalog, setup/idempotence/uninstall, exact-release update, read-only maintenance and final executable assertions passed. This instrumented sample describes its executed source and runner; it is not a controlled speedup comparison with the historical 222s/558s steps or a claim about newer main 93e75bc8f.

The updater proxy returned 193 successful metadata responses / 60,622,461 decoded payload bytes / 0 tarball responses. The sum of individual upstream request durations was 10.99 s, with overlapping requests; this sum is not wall time, and decoded bytes are not compressed wire traffic. Responses finished within an observed 6.03 s window. These measurements do not identify the CPU/filesystem/network cause of the remaining npm time. Hosted process-family CPU/RSS and agent tokens remain unmeasured.

Two tempting explanations did not justify a fix:

The actual Windows verifier inherited an explicit npm cache configuration. Changing HOME alone therefore does not establish a cache miss.
A successful, real pinned npm 11.16.0 global installation against a controlled registry requested Accept: application/json. The pinned Arborist manifest path explicitly requests full metadata. Although the registry supports abbreviated metadata, assuming this client requests it was wrong. The first investigator assertion expecting abbreviated Accept failed; retaining the actual request and checking the pinned source refuted that hypothesis. This was not a product/test regression. Forcing abbreviated responses would change the workload rather than establish a faithful optimization.

Runner-delay check on the two historical same-tree samples: Windows started 4s (main) / 3s (PR) after its last required dependency completed. That combined dispatch/scheduling gap does not support blaming a long Windows-runner queue in these samples; it does not measure unrelated queues.

Stop this round without deleting distinct guarantees, caching live passes, changing timeouts, or proposing an unproven performance patch. The temporary profiling workflow is not an integration PR. Remaining npm processing cost requires discriminating CPU/I/O evidence before another implementation round. The two separately reproduced correctness defects are merged as #3549 and #3565; #3543 remains open after a non-reproducing macOS timing experiment.

One separate historical observation remains unrooted: qualification run 38590708469, job 115944977400, failed cliOutput.test.ts / retains closed slices and multiple filter paths at its 3,000 ms owned-child deadline. The gate correctly rejected that failure. There is no evidence that it shares #3543's cause, and later feedback success is not a fix for this historical full/coverage sample. No timeout change or retry was applied; retain this observation for a future discriminating reproduction.

Final newer-main follow-up: 93e75bc8fe21780ab1ee517bfda4f4a7730d678d CI 38602581852 completed successfully, 13 selected code jobs passed / 17 unselected jobs skipped. Downloaded exact-head report: 1,285 unique files / 14,680 passed / 125 skipped / 0 failed / 0 duplicates. Local affected checks on this newer main: four files / 34 passed, eight static tasks, architecture guards over 1,398 source files, all 1,285 files discovered exactly once. These results do not rerun the historical failing qualification environment or prove its root cause.

@N0zoM1z0 commented on GitHub (Oct 14, 2026): Bounded Windows profile completed successfully: [run 38602408204](https://github.com/morluto/rea/actions/runs/38602408204), exact diagnostic head `15f44d640d4910051016851f194d4161a1e3a01c`, production baseline main `1579b6865`. Both jobs passed; the native artifact was built from that same run/head. Artifact `windows-package-phase-profile`: ID **11915231329**. | Actual verifier phase | Wall seconds | | --- | ---: | | pack, including its normal prepack checks | 5.92 | | fresh local installation | 122.02 | | CLI help | 1.22 | | MCP catalog and lifecycle | 10.84 | | setup / idempotence | 2.51 / 1.83 | | uninstall | 1.32 | | update total | 117.84 | | updater's real npm installation (inside update total) | 109.46 | | cleanup | 8.54 | | complete traced interval | **272.24** | All original package/native-file, complete MCP catalog, setup/idempotence/uninstall, exact-release update, read-only maintenance and final executable assertions passed. This instrumented sample describes its executed source and runner; it is not a controlled speedup comparison with the historical 222s/558s steps or a claim about newer main `93e75bc8f`. The updater proxy returned **193 successful metadata responses / 60,622,461 decoded payload bytes / 0 tarball responses**. The sum of individual upstream request durations was 10.99 s, with overlapping requests; this sum is not wall time, and decoded bytes are not compressed wire traffic. Responses finished within an observed 6.03 s window. These measurements do not identify the CPU/filesystem/network cause of the remaining npm time. Hosted process-family CPU/RSS and agent tokens remain unmeasured. Two tempting explanations did **not** justify a fix: - The actual Windows verifier inherited an explicit npm cache configuration. Changing HOME alone therefore does not establish a cache miss. - A successful, real pinned **npm 11.16.0 global installation** against a controlled registry requested `Accept: application/json`. The [pinned Arborist manifest path explicitly requests full metadata](https://github.com/npm/cli/blob/v11.16.0/workspaces/arborist/lib/arborist/build-ideal-tree.js#L1288). Although the [registry supports abbreviated metadata](https://github.com/npm/registry/blob/main/docs/responses/package-metadata.md), assuming this client requests it was wrong. The first investigator assertion expecting abbreviated Accept failed; retaining the actual request and checking the pinned source refuted that hypothesis. This was not a product/test regression. Forcing abbreviated responses would change the workload rather than establish a faithful optimization. Runner-delay check on the two historical same-tree samples: Windows started **4s** (main) / **3s** (PR) after its last required dependency completed. That combined dispatch/scheduling gap does not support blaming a long Windows-runner queue in these samples; it does not measure unrelated queues. Stop this round without deleting distinct guarantees, caching live passes, changing timeouts, or proposing an unproven performance patch. The temporary profiling workflow is not an integration PR. Remaining npm processing cost requires discriminating CPU/I/O evidence before another implementation round. The two separately reproduced correctness defects are merged as #3549 and #3565; #3543 remains open after a non-reproducing macOS timing experiment. One separate historical observation remains unrooted: qualification run [38590708469](https://github.com/morluto/rea/actions/runs/38590708469), job 115944977400, failed `cliOutput.test.ts` / `retains closed slices and multiple filter paths` at its **3,000 ms owned-child deadline**. The gate correctly rejected that failure. There is no evidence that it shares #3543's cause, and later feedback success is not a fix for this historical full/coverage sample. No timeout change or retry was applied; retain this observation for a future discriminating reproduction. Final newer-main follow-up: `93e75bc8fe21780ab1ee517bfda4f4a7730d678d` CI [38602581852](https://github.com/morluto/rea/actions/runs/38602581852) completed successfully, **13 selected code jobs passed / 17 unselected jobs skipped**. Downloaded exact-head report: **1,285 unique files / 14,680 passed / 125 skipped / 0 failed / 0 duplicates**. Local affected checks on this newer main: four files / 34 passed, eight static tasks, architecture guards over 1,398 source files, all 1,285 files discovered exactly once. These results do not rerun the historical failing qualification environment or prove its root cause.

bsakaguchi referenced this issue

2026-10-16 21:15:36 +00:00

[PR #732] [MERGED] fix(process): scope comparison unknowns to their evidence #2200

bsakaguchi referenced this issue

2026-10-16 21:15:37 +00:00

[PR #734] [MERGED] feat(evidence): accept exact retained application references #2204

bsakaguchi added

enhancement

and removed

enhancement

labels

2026-10-31 00:29:30 +00:00