Skip to content

test(configuration): bound harness resolution against a clone of the same document - #2414

Merged
freeqaz merged 3 commits into
mainfrom
fix-1326
Oct 14, 2026
Merged

freeqaz merged 3 commits into
mainfrom
fix-1326

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

The Harness resolution test from #2401 (50k fallbacks against a 50k-row catalog, and 50k entries sharing 50k fallbacks) asserts an absolute 1 s per resolution. In CI the whole test has reached 0.89 s of wall time (run 38545875684), and a contended local run takes 600-915 ms per call. A slow or busy runner would fail it (finding 1326).

Change

Tests only. The bound now compares resolution with a structuredClone of the same document, timed in the same run, as #2405 does for the large roster test. Each round times one resolution and then one clone. There are up to three rounds, and each time keeps its best. Resolution must stay under 50 clones. A round past 500 clones is the regression, not noise, so the test stops retrying and fails at once. Every round still checks the resolved Harness.

Verification

All runs below are pinned to 2 CPUs (taskset -c 0,1).

Risks

Low, because this changes one test only. If a busy runner makes a clone look faster than resolution, a slow resolution could pass. Even so, the quadratic regression is 100 times past the bound.

…same document

The #2401 test asserted an absolute 1 s per resolution. CI reached 0.89 s of
test wall and a contended local run 600-915 ms per call, so a slow runner would
fail it (finding 1326). Time a structured clone of the same document in the same
run and require resolution under 25 clones, best of up to three rounds; a miss
past 250 clones is the regression and is not retried. The reverted indexing
fails at about 5,800 clones (123 s); at 2,000 models it already fails at about
340.
With four copies of the test sharing two CPUs a single round reached 17.7
clones (p50 4.6, 160 samples) against the bound of 25. Allow 50 and stop
retrying past 500; the reverted indexing still fails at about 320 clones with
2,000 models and about 5,800 at full size.
…inly

Review nits: the 4-copy rerun peaked at 19.7 clones, the shrunk mutant figure
is about 320 with 2,000 models, and the loop stops once under the bound.
@freeqaz
freeqaz requested a review from a team as a code owner October 14, 2026 11:03
@clawsweeper

clawsweeper Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 14, 2026
@clawsweeper

clawsweeper Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR replaces a one-second Harness-resolution deadline with a bound relative to cloning the same document.

Example: Resolve 50,000 model fallbacks against a 50,000-row catalog on a busy runner.

  • Before: Resolution fails the test whenever it exceeds 1,000 ms.
  • After: Resolution must take less than 50 times the measured clone duration, with up to three measurement rounds.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and has no identified defect; confidence rests on source inspection and reported unit-level measurements, with current CI validation incomplete.
Proof confidence 🦐 gold shrimp (3/6) This test-only change has no changed shipped behavior requiring live proof; the supplied contention and mutation measurements concern the test and were not independently executed during this read-only review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Busy CI runners can approach an absolute performance-test deadline despite retaining the intended linear resolver behavior.
Reason: A focused adjustment preserves valuable regression coverage and reduces hardware-dependent failures without changing runtime contracts.

Merge readiness

✅ Ready for maintainer review

Keep open: this focused test repair reduces runner-speed sensitivity while retaining the original regression cases; no actionable patch defect was found.

Priority: P3
Reviewed head: df7224913f004af17465c834629e11c6a2bfd3be

Before merge

None.

Findings

None.

Agent review details

How this fits together

OCC resolves Configuration model policies into a Harness identity or a validation error; this conformance test guards resolution cost for large catalogs and shared fallback selections.

flowchart TB
  A[Configuration document] --> B[Harness resolver]
  B --> C[Harness identity or refusal]
  D[Conformance test] --> B
  D --> E[Clone same document]
  B --> F[Relative timing assertion]
  E --> F
Loading

Technical review

Best possible solution:

Retain the existing regression inputs and semantic assertions while using the calibrated relative timing bound.

Do we have a high-confidence way to reproduce the issue?

The absolute deadline is verified in current main, but a runner-induced failure was not reproduced; the PR reports near-bound measurements and contention experiments.

Is this the best way to solve the issue?

The change is proportionate: it preserves independent Harness assertions, follows an existing relative-baseline pattern, and reports mutation evidence that the original quadratic regression still fails.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against ea23d9025c42.

Provenance checked

  • Harness-resolution performance assertion changes intended behavior with a stated reason (7b6f35c: Detect catalog-by-selection and shared-fallback expansion regressions that previously stalled the controller or exhausted memory.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 7b6f35c adds tests/conformance/configuration-occ.test.mjs:748 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 7b6f35c8ddca; files: tests/conformance/configuration-occ.test.mjs)

Review metrics

Metric Value Why it matters
Change distribution Production +0/-0; tests +27/-6 Growth is confined to baseline measurement and bounded retries in one existing regression test.

Labels

Label changes:

  • add P3: This improves test reliability without changing product behavior.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This improves test reliability without changing product behavior.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 14, 2026, 7:06 AM ET / 11:06 UTC.

@freeqaz
freeqaz merged commit df24287 into main Oct 14, 2026
74 of 78 checks passed
@freeqaz
freeqaz deleted the fix-1326 branch October 14, 2026 11:19

This branch had an error being deployed

1 failed deployment
integration-qa-pr — df722491 Deployed Oct 14, 2026 by freeqaz via QA advisory (compose) #851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant