test(profile): Pin the truncation lookahead boundary

[sujoydc]

What

Test-only follow-up to #585, addressing @haofeif's nit
(#585 (comment)).

The schema-error sampler in profile_validator.py takes
islice(iter_errors(...), remaining + 1). That + 1 lookahead is
load-bearing: the extra entry is the only evidence an omitted tail
existed, so len(sampled_errors) > remaining is what makes truncation
detectable without draining the iterator.

It's also an off-by-one that fails in the quiet direction. Drop the
+ 1 and every visible finding stays correct — truncation just silently
stops being reported. No existing test would go red.

Changes

Two boundary cases in test/services/test_profile_validator.py, using
a deterministic fake validator (same pattern as the existing
iterator-consumption test):

• exactly _MAX_FINDINGS - 1 schema errors → all surface, no omission
  marker (the lookahead peeks one past the budget, finds nothing, stays
  silent)
• exactly _MAX_FINDINGS schema errors → marker present exactly once,
  last, at error severity

Validation

• Mutation-checked: removing the + 1 from the sampler fails the second
  test immediately, so the pin is real rather than vacuous
• test/services/test_profile_validator.py: 65 passed
• black, isort, git diff --check clean

No production code changes.

[haofeif]

Thank you @sujoydc for turning my nit around so quickly! Reviewed at d2cd0d9. Test-only, no production change, 149 passed. Approving — but the two tests are not equally load-bearing, and the reason given for the second one is wrong. That's my fault, not yours: you inherited a mistake I made in the nit.

Mutation results

I didn't take the mutation claim on trust — I ran a battery against the sampler and recorded which tests actually catch each one:

mutation	caught by
M1 remaining + 1 → remaining	test_schema_iterator_consumes_only_remaining_plus_one (pre-existing), test_schema_prefix_is_stable_across_hash_seeds (pre-existing), test_one_error_past_the_budget_emits_exactly_one_marker (new)
M2 len(...) > remaining → >= remaining	test_exactly_filling_the_budget_emits_no_marker (new) — and nothing else
M3 drop [:remaining] slice	pre-existing only
M4 lookahead of 2	pre-existing only

Correcting my own nit

My nit asserted that dropping the + 1 would leave "no test going red." That was wrong, and you quite reasonably carried it into the PR body as "No existing test would go red."

test_schema_iterator_consumes_only_remaining_plus_one — added by #585 itself, in this same file, and present at 4a6e8fde, the exact head I was reviewing when I wrote the nit — already pins it directly:

assert consumed == remaining_after_key_findings + 1

I should have grepped the file before claiming the gap existed. Apologies for sending you after a hole that was already plugged.

The PR is still worth merging, for the other test

The half of the nit that was right is test_exactly_filling_the_budget_emits_no_marker, and it turns out to be the more valuable one. I checked out main's version of the test file, applied M2 to the sampler, and ran main's suite unmodified:

M2 (> becomes >=) against MAIN's suite only:  63 passed

The mutation survives every test on main. With your file it fails immediately. So the spurious-marker direction — a document that exactly fills the budget being told findings were omitted when none were — was genuinely uncovered, and this test is the only thing standing in front of it. That is a real regression risk closed, and it happens to be the quieter of the two failure modes: a truncation marker on a complete result is the kind of thing nobody files a bug about.

test_one_error_past_the_budget_emits_exactly_one_marker is largely redundant against M1, but I would still keep it — see the inline note.

Other checks

• The fake is faithful to the interface the sampler uses. FakeError supplies path, absolute_path, and message, which is exactly what sorted(..., key=lambda e: [str(p) for p in e.path]) and the absolute_path join consume, so neither test passes for a structural reason.
• Zero-padding in field{index:04} is a nice catch — without it field10 sorts before field9 and the fixture's ordering stops matching its own intent.
• Both tests are hash-seed stable (PYTHONHASHSEED=0/1/42, 2 passed each), which matters given the class they live in is about prefix determinism.
• Both derive from _MAX_FINDINGS rather than hard-coding 99/100, so retuning the budget won't strand them.
• Worktree confirmed clean after my mutation runs; nothing here touches production code.

[haofeif]

[nit] This one is mostly redundant — worth re-pointing its docstring at what it uniquely holds.

The docstring says dropping the + 1 "would silently stop reporting truncation," which is the justification I gave you and which turns out to be inaccurate: test_schema_iterator_consumes_only_remaining_plus_one, ~60 lines above in this same class, already fails on that mutation via assert consumed == remaining_after_key_findings + 1. So does test_schema_prefix_is_stable_across_hash_seeds.

I'd still keep the test — it asserts the observable contract (marker present, exactly once, last, error severity) where the older one asserts iterator mechanics, and those are worth pinning separately. But as written, someone auditing coverage later will read this docstring, believe it is the sole guard on the + 1, and may delete the older test as duplicative — which would drop the consumed assertion, the only check that the tail isn't drained.

Something like:

"""The omission marker's shape at the smallest truncating input.

Complements test_schema_iterator_consumes_only_remaining_plus_one:
that one pins how much of the iterator is consumed, this one pins
what the caller actually sees — exactly one marker, last, at error
severity.
"""

No change needed to the assertions themselves; they're right.

For contrast, test_exactly_filling_the_budget_emits_no_marker above genuinely stands alone — I confirmed > → >= survives all 63 tests on main and is caught only by it. That one's docstring is accurate as written.