Skip to content

test: cap _SiteStats.record shape and caller sets (PRPUNDIT-13) - #1328

Merged
chaojhou merged 4 commits into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-13
Sep 1, 2026
Merged

test: cap _SiteStats.record shape and caller sets (PRPUNDIT-13)#1328
chaojhou merged 4 commits into
AMD-AGI:mainfrom
jiagaoxiang:testgap/PRPUNDIT-13

Conversation

@jiagaoxiang

Copy link
Copy Markdown
Collaborator

Summary

  • Calls _SiteStats.record past _MAX_CONTAINER_WIDTH / _MAX_CALLERS_PER_SITE and asserts count/wall/nbytes and first/last keep moving while the sets stop growing.

Closes test gap PRPUNDIT-13.

Test plan

  • PYTHONPATH=src pytest src/hyperloom/inference_optimizer/tests/test_framework_rewrite_evidence.py::test_site_stats_record_caps_shape_sigs_and_callers_while_tallies_grow

Call record() past _MAX_CONTAINER_WIDTH/_MAX_CALLERS_PER_SITE and assert tallies still grow while the sets stop.
@jiagaoxiang
jiagaoxiang requested a review from a team as a code owner August 28, 2026 07:13

@zoroyihan7 zoroyihan7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the caps are solidly pinned: pushing past both bounds, asserting the counters keep accumulating, and explicitly asserting the overflow entries are absent from the sets is exactly the right shape, and removing either len(...) < CAP condition turns this test red.

One gap: first_s is only half-pinned. Mutating the sentinel init at hl_host_probe.py:223:

self.first_s = 0.0      # was: -1.0

kills the latch outright — if self.first_s < 0 never fires again, so first_s is permanently 0.0 and never records the real first timestamp — and all 73 tests stay green.

The reason is that the loop's first call passes at_s=0.0, which is exactly the degenerate value, so assert stats.first_s == 0.0 can't distinguish "latched the first timestamp" from "never latched anything". (Deleting the if so first_s is overwritten every call is caught — that's the other half.)

One-token fix: drive the loop with at_s=float(i) + 1.0 and assert first_s == 1.0. That kills both the sentinel mutation and the overwrite mutation.

Two nits while you're in here:

  • The falsy-input branch of the guards (if shape_sig and ... / if caller and ..., lines 242/244) isn't pinned — replacing them with bare if len(...) < CAP: leaves the file green. Passing shape_sig="" / caller="" and asserting the empty string is absent would be two more lines. Outside the contract this PR claims, so genuinely optional.
  • Re-asserting first_s after the post-cap record would make the latch/overwrite contrast explicit next to the last_s == 99.0 assertion.

@zoroyihan7 zoroyihan7 added the skip-e2e-test It's a PR that doesn't need to be e2e tested label Aug 31, 2026
jiagaoxiang and others added 2 commits August 31, 2026 19:48
… mutation

With at_s=0.0 on the first record call, first_s==0.0 cannot distinguish 'latched
the real first timestamp' from 'sentinel initialised to 0.0 and never latched'.
Using at_s=float(i)+1.0 means first_s must equal 1.0, killing:
  - sentinel-init mutation (self.first_s = 0.0): latch never fires, first_s stays 0.0
  - always-overwrite mutation (no if guard): first_s gets clobbered each iteration

Addresses zoroyihan7 review on PR AMD-AGI#1328.
Post-cap last_s==99.0 now sits next to first_s==1.0 so the latch cannot
be confused with overwrite. Empty shape_sig/caller are recorded while
the sets still have room, pinning the falsy `if shape_sig and` guards.
@jiagaoxiang

Copy link
Copy Markdown
Collaborator Author

at_s=float(i)+1.0 with first_s == 1.0 pins the sentinel-init mutation. Re-asserted first_s == 1.0 after the post-cap last_s == 99.0 record, and added a case that records empty shape_sig/caller while the sets still have room.

@zoroyihan7 zoroyihan7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed. The at_s offset closes the sentinel hole.

Mutations red now:

  • self.first_s = -1.00.0 in _SiteStats.__init__ (L223), which kills the latch outright — red (2 tests)
  • if self.first_s < 0: → always true, i.e. overwrite on every call — red
  • if shape_sig and guard dropped (L242) — red
  • if caller and guard dropped (L244) — red

Both halves of the latch contract are pinned now. 74 passed. LGTM.

FYI, not for this PR: the identical sentinel at L290 belongs to a different class (depth / arg_samples / strict_sigs) and stays green under the same mutation. Might be worth its own ticket.

@jiagaoxiang

Copy link
Copy Markdown
Collaborator Author

Leaving the _CallStats sentinel (hl_host_probe.py:290) out of this PR as discussed. Filed #1356.

@jiagaoxiang
jiagaoxiang enabled auto-merge (squash) September 1, 2026 03:35
@zoroyihan7 zoroyihan7 added the retest Re-run E2E smoke without a new commit (runs once) label Sep 1, 2026
@chaojhou
chaojhou disabled auto-merge September 1, 2026 09:42
@chaojhou
chaojhou merged commit 8a8f8bd into AMD-AGI:main Sep 1, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

retest Re-run E2E smoke without a new commit (runs once) skip-e2e-test It's a PR that doesn't need to be e2e tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants