Skip to content

fix(camera): report the nominations the stall report could not see - #225

Merged
cbrightly merged 1 commit into
mainfrom
fix/stall-report-sees-every-nomination
Aug 9, 2026
Merged

fix(camera): report the nominations the stall report could not see#225
cbrightly merged 1 commit into
mainfrom
fix/stall-report-sees-every-nomination

Conversation

@cbrightly

Copy link
Copy Markdown
Owner

Follow-up to #224, which merged while this was being checked. It is a
correctness fix to the report's data source, not a new feature: #224 as
merged under-reports nomination on precisely the shape the run is meant to
diagnose.

What is wrong in #224 as merged

The report sourced nominated= and use-candidate= from _bridge_uc_info
alone. That dict is fed by the setup parse, the trickle consumer, the
late-answer parses and the bridge's peer-reflexive learning - but NOT by
_nominate_from_answer_sdp, which is the function that actually nominates on
a late answer. It parses the peeked answer, installs permissions, sends
USE-CANDIDATE to every candidate and returns a count; it never touches
_bridge_uc_info["cands"] and never flips ["sent"].

A late answer is exactly the measured failure shape - item 3 records the
answer landing at +1.3 s, missing the _pre_launch_answer_sdp snapshot. So
on that shape the line would have said:

... nominated=none; use-candidate=not-sent; binding-success=0; trigger=not-sent; ...

on an open that nominated one candidate and probed it. Two consequences, both
fatal to the experiment:

  • the report claims nothing was nominated when something was; and
  • the item-3 table's "answer carried no ICE credentials" row becomes
    indistinguishable from its "answer carried a single unroutable candidate"
    row. Those are different diagnoses and telling them apart is half of what
    the run is for.

Same class of error as the _br_stun_resp_count one already called out in
#224: a field that reads plausible and answers a different question.

The fix

_nominate_from_answer_sdp records what it nominated into _nominated_seen,
and the report takes the deduped, order-preserving union of that, the
_bridge_uc_info set and the peer-reflexive set. Any one source alone
under-reports on some shape. use-candidate is now true if either path sent
one.

Two smaller ones in the same commit

The dropped-probe count was packets, not sources. _br_probe_overflow
incremented per packet once the cap was hit, so one unrecorded source probing
fifty times rendered as (+50 more source(s)) - a wrong number in the
deliverable line that someone would reason from. It is a set of distinct
source labels now.

The gating test was weaker than its name. It searched for the bare
substring _first_video_pt[0] is None, which the while condition above the
guard also contains, so it would have passed with the if deleted. It
anchors on the dedented if now. The guard itself was correct; only the test
was.

Still instrumentation only, still one WARNING on the stall path, still
nothing new above DEBUG on a healthy open. Two new source-level tests, 25 in
the file. Full unit tier green (1082 passed, 5 skipped);
ruff check aidot_cameras/ tests/ clean.

Sourcing the report's nominated set from _bridge_uc_info alone was wrong on
exactly the shape that stalls. On a late answer - measured at +1.3 s, so it
misses the pre-launch snapshot - it is _nominate_from_answer_sdp inside the
first-media wait that nominates, and that function neither appends to
_bridge_uc_info["cands"] nor flips ["sent"]. The line would have printed
"nominated=none; use-candidate=not-sent" on an open that did nominate, and
collapsed the "answer carried no ICE credentials" row into the "answer
carried one unroutable candidate" row - two different diagnoses, and the
whole reason the run exists.

Record what that path nominated and union it with the setup / trickle /
late-answer set and the bridge's peer-reflexive learning, deduped and
order-preserving. Any one source alone under-reports on some shape.

Also make the dropped-probe count distinct sources rather than packets: one
unrecorded source probing fifty times is one source, and a per-packet counter
would have put a wrong number in the deliverable line.

The gating test now anchors on the dedented `if` rather than the bare
substring - the `while` condition above contains the same text, so the old
form would have passed with the guard deleted.
@cbrightly
cbrightly merged commit de8b0b3 into main Aug 9, 2026
16 checks passed
@cbrightly
cbrightly deleted the fix/stall-report-sees-every-nomination branch August 9, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant