h-dv/code-index

Watch

Fork 0

`claim_domain_scale` moved ONE wall-clock ratio to `#[ignore]` and left its sibling live — the sibling is a FLOOR, which contention breaks in the direction a floor cannot survive #253

New issue

Closed

opened 2026-09-10 09:07:03 +02:00 by buildagent · 2 comments

buildagent commented

2026-09-10 09:07:03 +02:00

Member

The failure

the_routing_scan_is_the_product_of_claims_and_keys_and_the_key_total_is_what_bounds_it
went RED in a local cargo test --workspace at load average 37.9 (a concurrent
session was running cargo in another worktree):

claim_domain_scale.rs:1260
the 4x domain scanned in 3930.83 ms against 2090.31 ms, so the scan did not grow
with the file count and the byte comparison below would be about two calls that did nothing

1.88×, against assert!(big_ms > small_ms * 2.0).

Re-run isolated at load 3.0 on the same tree: PASSES (4 passed; 0 failed; 1 ignored,
5.70 s versus 8.80 s contended). CI's cargo test job also passed it at bd1c4e2. So it
is contention, not a regression — but the failure message says "the scan did not grow
with the file count", which is a claim about the product, and that is the wrong verdict
to hand a reader.

Why this is a finding and not just flakiness

This file has already been here, wrote it up carefully, and fixed the OTHER instance.
From its own module doc:

AND THE ONE WALL-CLOCK ASSERTION THIS FILE KEPT IS NOW #[ignore]d […]
MEASURED, on a release sweep: 9.06x, red, with no code change — MCP calls were
hitting the daemon while ~190 test binaries ran. Re-run isolated on the same commit:
passes 3/3.

WHY BEING A RATIO DID NOT SAVE IT. The doc argued that "whatever the machine is
doing to one it is doing to the other". That is true of two measurements taken
SIMULTANEOUSLY and false of two taken in sequence, which is what these are: a
scheduling spike lands wholly on whichever arm was running.

the_cost_of_a_claim_domain_is_linear_in_the_files_it_holds was therefore moved to
#[ignore], executed by the weekly plugin-path-cost job on an idle runner, and
registered in release_gate.rs::TIMING_GATES so the #[ignore] could not quietly become
a measurement nothing runs.

The sibling three hundred lines below was left live, with the identical construction:
two sequential Instant::now() arms inside cargo test --workspace.

And the direction matters. The quoted paragraph describes a spike landing on the
NUMERATOR, inflating a ratio and breaking a ceiling. This one is a floor, so the
spike that breaks it lands on the DENOMINATOR — small_ms runs first and absorbs the
contention, compressing the ratio toward 1. Same mechanism, opposite arm, and a floor is
exactly what it kills.

The obvious fix does not work, and the test says why

My first instinct was to assert on the machine-independent unit this file itself
establishes — bytes allocated, which it measured as byte-identical across opt-level=0
and lto=thin while the wall clock differed 25x.

That is refuted by the test's own final assertion:

assert_eq!(b_small, b_big, "four times the routing scan allocated {b_small} B and then {b_big} B — it is no longer invisible to the byte counter …");

The bytes are equal by design: the whole point of the case is that the routing scan is
invisible to the byte counter. That is the fact being recorded. So bytes cannot be the
anti-vacuity guard here, and measure() returns only (value, bytes, peak) — both
allocation measures — so the fixture offers no other machine-independent signal. The time
assertion is there because it was the only thing left.

Options

Follow the file's own precedent: #[ignore] + register in
release_gate.rs::TIMING_GATES + run it on the weekly idle-runner job, exactly as its
sibling was handled. Cheapest, consistent, and needs no new instrumentation.
Count the work instead of timing it: a Key::matches call counter behind a
test-only cfg would be a property of the code and the data, which is the requirement
the module doc states. More faithful to the intent, more invasive.

Not folded into the change that found it: the failing test is unrelated to that diff
(it exercises crates/indexer/src/dirty.rs, and the diff touches no src/ at all), and
picking between the two options is a decision this file has earned the right to make
deliberately.

## The failure `the_routing_scan_is_the_product_of_claims_and_keys_and_the_key_total_is_what_bounds_it` went RED in a local `cargo test --workspace` at **load average 37.9** (a concurrent session was running cargo in another worktree): ``` claim_domain_scale.rs:1260 the 4x domain scanned in 3930.83 ms against 2090.31 ms, so the scan did not grow with the file count and the byte comparison below would be about two calls that did nothing ``` 1.88×, against `assert!(big_ms > small_ms * 2.0)`. **Re-run isolated at load 3.0 on the same tree: PASSES** (`4 passed; 0 failed; 1 ignored`, 5.70 s versus 8.80 s contended). CI's `cargo test` job also passed it at `bd1c4e2`. So it is contention, not a regression — but the failure message says *"the scan did not grow with the file count"*, which is a claim about the product, and that is the wrong verdict to hand a reader. ## Why this is a finding and not just flakiness This file has already been here, wrote it up carefully, and fixed the OTHER instance. From its own module doc: > **AND THE ONE WALL-CLOCK ASSERTION THIS FILE KEPT IS NOW `#[ignore]`d** […] > MEASURED, on a release sweep: **9.06x**, red, with no code change — MCP calls were > hitting the daemon while ~190 test binaries ran. Re-run isolated on the same commit: > passes 3/3. > > **WHY BEING A RATIO DID NOT SAVE IT.** The doc argued that "whatever the machine is > doing to one it is doing to the other". That is true of two measurements taken > SIMULTANEOUSLY and false of two taken in sequence, which is what these are: a > scheduling spike lands wholly on whichever arm was running. `the_cost_of_a_claim_domain_is_linear_in_the_files_it_holds` was therefore moved to `#[ignore]`, executed by the weekly `plugin-path-cost` job on an idle runner, and registered in `release_gate.rs::TIMING_GATES` so the `#[ignore]` could not quietly become a measurement nothing runs. **The sibling three hundred lines below was left live**, with the identical construction: two sequential `Instant::now()` arms inside `cargo test --workspace`. And the direction matters. The quoted paragraph describes a spike landing on the NUMERATOR, inflating a ratio and breaking a *ceiling*. This one is a **floor**, so the spike that breaks it lands on the DENOMINATOR — `small_ms` runs first and absorbs the contention, compressing the ratio toward 1. Same mechanism, opposite arm, and a floor is exactly what it kills. ## The obvious fix does not work, and the test says why My first instinct was to assert on the machine-independent unit this file itself establishes — bytes allocated, which it measured as *byte-identical across `opt-level=0` and `lto=thin` while the wall clock differed 25x*. That is refuted by the test's own final assertion: ```rust assert_eq!(b_small, b_big, "four times the routing scan allocated {b_small} B and then {b_big} B — it is no longer invisible to the byte counter …"); ``` The bytes are **equal by design**: the whole point of the case is that the routing scan is invisible to the byte counter. That is the fact being recorded. So bytes cannot be the anti-vacuity guard here, and `measure()` returns only `(value, bytes, peak)` — both allocation measures — so the fixture offers no other machine-independent signal. The time assertion is there because it was the only thing left. ## Options 1. **Follow the file's own precedent**: `#[ignore]` + register in `release_gate.rs::TIMING_GATES` + run it on the weekly idle-runner job, exactly as its sibling was handled. Cheapest, consistent, and needs no new instrumentation. 2. **Count the work instead of timing it**: a `Key::matches` call counter behind a test-only cfg would be a property of the code and the data, which is the requirement the module doc states. More faithful to the intent, more invasive. Not folded into the change that found it: the failing test is unrelated to that diff (it exercises `crates/indexer/src/dirty.rs`, and the diff touches no `src/` at all), and picking between the two options is a decision this file has earned the right to make deliberately.

buildagent referenced this issue

2026-09-10 09:51:05 +02:00

link_payload_scaling_e2e's per-link ceiling grades the TEMP DIRECTORY'S LENGTH — 59 tokens on /tmp, 80 on a long path, 63 on the Windows runner #252

buildagent commented

2026-09-10 12:37:43 +02:00

Author

Member

The likelihood of this firing went up materially today, and the mechanism is measurable. Adding it here because it changes the urgency without changing the decision.

3aea49d un-broke OSS corpus (tier 1). That job had been aborting at corpus_stage around 25 min; it now runs everything and takes ~54 min. cargo test shares the runner pool:

run	corpus job	`cargo test`
722 / 724 `bd1c4e2`	25m (failed early)	32m
728 `5131654`	54m (succeeds)	68m

So the per-push window in which a sequential wall-clock ratio is measured under contention has roughly doubled. A sibling test in a different file — xaml_large_file_budgets::the_fact_buffer_ceiling_fires_and_says_so — failed on that very run for the same underlying reason (a timing bound beaten by a loaded machine), which is #254.

Checked, so the next person does not have to

Registration is not a blocker for option 1. release_gate::TIMING_GATES addresses the binary:

"claim_domain_scale",  …  "--test claim_domain_scale"

and ci.yml's weekly job runs --test claim_domain_scale -- --ignored --nocapture --test-threads=1, which picks up all ignored tests in that binary. ignored_test_reachability sweeps the population and demands each ignored test be reachable or waived — a second #[ignore] here is reachable through the existing invocation, so it needs no new plumbing.

But option 1 costs more than it looks, which is why I did not just do it

The wall-clock assertion is a precondition for the one that follows it:

assert!(big_ms > small_ms * 2.0, "…the byte comparison below would be about two calls that did nothing");
assert_eq!(b_small, b_big, "…");

#[ignore] moves the whole test to the weekly runner, so per-push loses the byte-equality assertion as well — not just its fragile guard. That is a real reduction in per-push coverage, traded for stability, and it is exactly the kind of trade that should be made deliberately rather than under CI pressure.

Option 2 (count Key::matches calls behind a test-only cfg) keeps both assertions on every push and replaces the fragile precondition with a property of the code and the data, which is what the module doc says the unit should be. It costs instrumentation.

I still think that is the better answer, and I am deliberately not deciding it as a side effect of a CI-stability push.

**The likelihood of this firing went up materially today, and the mechanism is measurable.** Adding it here because it changes the urgency without changing the decision. `3aea49d` un-broke `OSS corpus (tier 1)`. That job had been aborting at `corpus_stage` around 25 min; it now runs everything and takes ~54 min. `cargo test` shares the runner pool: | run | corpus job | `cargo test` | |---|---|---| | 722 / 724 `bd1c4e2` | 25m (failed early) | 32m | | 728 `5131654` | 54m (succeeds) | **68m** | So the per-push window in which a sequential wall-clock ratio is measured under contention has roughly doubled. A sibling test in a different file — `xaml_large_file_budgets::the_fact_buffer_ceiling_fires_and_says_so` — failed on that very run for the same underlying reason (a timing bound beaten by a loaded machine), which is #254. ## Checked, so the next person does not have to Registration is **not** a blocker for option 1. `release_gate::TIMING_GATES` addresses the *binary*: ``` "claim_domain_scale", … "--test claim_domain_scale" ``` and ci.yml's weekly job runs `--test claim_domain_scale -- --ignored --nocapture --test-threads=1`, which picks up **all** ignored tests in that binary. `ignored_test_reachability` sweeps the population and demands each ignored test be reachable or waived — a second `#[ignore]` here is reachable through the existing invocation, so it needs no new plumbing. ## But option 1 costs more than it looks, which is why I did not just do it The wall-clock assertion is a *precondition* for the one that follows it: ```rust assert!(big_ms > small_ms * 2.0, "…the byte comparison below would be about two calls that did nothing"); assert_eq!(b_small, b_big, "…"); ``` `#[ignore]` moves the **whole test** to the weekly runner, so per-push loses the byte-equality assertion as well — not just its fragile guard. That is a real reduction in per-push coverage, traded for stability, and it is exactly the kind of trade that should be made deliberately rather than under CI pressure. Option 2 (count `Key::matches` calls behind a test-only cfg) keeps both assertions on every push and replaces the fragile precondition with a property of the code and the data, which is what the module doc says the unit should be. It costs instrumentation. I still think that is the better answer, and I am deliberately not deciding it as a side effect of a CI-stability push.

dhoyer referenced this issue from a commit

2026-09-10 18:36:36 +02:00

fix: the routing scan's anti-vacuity guard is counted, not timed (#253)

buildagent referenced this issue from a commit

2026-09-11 11:17:06 +02:00

fix: bind query evidence and Python imports to proven index state

buildagent referenced this issue

2026-09-11 11:17:22 +02:00

Bind query evidence and Python imports to proven index state #264

buildagent commented

2026-09-11 20:24:42 +02:00

Author

Member

Released in v0.28.1, build 340a75a, via #264 and #265. Release CI passed, including the Windows archive round-trip smoke test. The routing regression now counts executed Key::matches comparisons around each scan. Both arms prove nonzero work and the 4x input proves 4x comparisons; allocation equality remains its separate assertion. The check stays in normal CI, with no contention-sensitive wall-clock floor and no widened tolerance. Workspace CI passed on Linux and Windows. Closing the deterministic-work fix.

Released in [v0.28.1](https://git.h-dv.de/h-dv/code-index/releases/tag/v0.28.1), build `340a75a`, via #264 and #265. [Release CI](https://git.h-dv.de/h-dv/code-index/actions/runs/752) passed, including the Windows archive round-trip smoke test. The routing regression now counts executed Key::matches comparisons around each scan. Both arms prove nonzero work and the 4x input proves 4x comparisons; allocation equality remains its separate assertion. The check stays in normal CI, with no contention-sensitive wall-clock floor and no widened tolerance. Workspace CI passed on Linux and Windows. Closing the deterministic-work fix.

buildagent closed this issue

2026-09-11 20:24:42 +02:00

~~buildagent referenced this issue 2026-09-11 20:25:15 +02:00~~

Prepare v0.28.2: Python receiver fix and historical acceptance #266

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

h-dv/code-index#253

No description provided.

Rows
Columns

claim_domain_scale moved ONE wall-clock ratio to #[ignore] and left its sibling live — the sibling is a FLOOR, which contention breaks in the direction a floor cannot survive #253

The failure

Why this is a finding and not just flakiness

The obvious fix does not work, and the test says why

Options

Checked, so the next person does not have to

But option 1 costs more than it looks, which is why I did not just do it

`claim_domain_scale` moved ONE wall-clock ratio to `#[ignore]` and left its sibling live — the sibling is a FLOOR, which contention breaks in the direction a floor cannot survive #253