Skip to content

Revise PR #250: measure REAL channel membership, not spool senders (Stage 2 entry gate) - #261

Closed
jaylfc wants to merge 2 commits into
masterfrom
exec/tsk-aildfj
Closed

Revise PR #250: measure REAL channel membership, not spool senders (Stage 2 entry gate)#261
jaylfc wants to merge 2 commits into
masterfrom
exec/tsk-aildfj

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 13, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Revise PR #250: measure REAL channel membership, not spool senders (Stage 2 entry gate)

Autonomous build of board card tsk-aildfj.

Files:
docs/specs/a2a-bus-auth-transition.md | 26 ++--
docs/specs/tsk-rf5gwb-membership-stems.md | 90 ++++++++++++
scripts/measure_membership_stems.py | 235 ++++++++++++++++++++++++++++++
tests/test_measure_membership_stems.py | 203 ++++++++++++++++++++++++++
4 files changed, 545 insertions(+), 9 deletions(-)

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 12 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 048a5b72-39ff-4bb6-9ac0-26ae4cfb0c10

📥 Commits

Reviewing files that changed from the base of the PR and between 051cf0a and 49d8214.

📒 Files selected for processing (4)
  • docs/specs/a2a-bus-auth-transition.md
  • docs/specs/tsk-rf5gwb-membership-stems.md
  • scripts/measure_membership_stems.py
  • tests/test_measure_membership_stems.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@jaylfc

jaylfc commented Aug 14, 2026

Copy link
Copy Markdown
Owner Author

The measurement is fixed and now reaches the opposite conclusion, which is the right one. BLOCKED on one thing: the numbers name no data source.

Reviewed on head 49d8214. 17 tests pass.

Every blocker from #250 is addressed

#250 blocker State on this branch
_collect_from_bus_spool was the default population Now _collect_from_archive is the default; --spool is opt-in and its own help string says "legacy, measures senders not membership"
Total distinct principals: 14 (spool senders, wrong field) 34, from archive EVENT_A2A rows, which is the membership population
Canonical entries with twin: 0 2
Status: CLOSED left stale in the doc Gone
fallback silently narrating Replaced: the tool prints No EVENT_A2A rows found and exits 1, verified by running it against an empty data dir (rc = 1)
is_install_discriminator dead code Deleted, and correctly so, see below

The conclusion flipped, and that is the point of the card. The verdict is a conditional
(if n3 == 0 and n4 <= 1), and with n3 = 2 it takes the else branch: "mint-stamp stripping
may NOT be safe for membership"
. #250 concluded the opposite off the wrong population. Stage 2
would have unlocked on that.

Deleting is_install_discriminator is the right call and it is tested

@taOS-dev's constraint from bus 2466 (normalisation must not collapse
@taOS-agent-<install8>) is now carried by the shape of the mint regex, which only matches a
trailing -YYYYMMDD-HHMMSS. That is pinned by two tests rather than left to inspection,
including the case that would actually catch a wrong regex:

stem_with_mint("@taOS-agent-abc12345-20260813-192605") == "taos-agent-abc12345"
test_install_discriminators_not_merged: two distinct installs -> multi_spell_stems_with_mint == {}

So the dead function is gone and the property it was supposed to guard is enforced and covered.

BLOCKER: the table that unlocks Stage 2 cannot be reproduced by anyone else

The doc's Method says "Point the tool at a taOSmd data dir that contains archive EVENT_A2A
rows"
and never says which one produced the 34 / 17 / 2 table. I tried to re-run it and
could not reach those numbers on any local data dir:

~/.taosmd                            -> No EVENT_A2A rows found, rc 1
~/Development/A2A/taosmd             -> 3 principals, canonical_twins 0
                                        CONCLUSION: mint-stamp stripping IS safe   <-- opposite verdict
~/Development/tinyagentos/data       -> no report

The A2A run is the control: the tool works, produces a full per-channel report, and exits 0. So
this is not a broken tool, it is an unidentified population. And note the second line: on a
different data dir the same script prints the opposite conclusion
, because the verdict is a
function of canonical_twins in whatever happened to be measured.

That matters more here than in an ordinary doc. This is the Stage 2 entry gate. A gate whose
headline number cannot be re-derived is a gate that has to be taken on trust, and the adjacent
claim "Run the command above twice on the same data dir and the table is identical" is not
checkable without knowing the dir either.

To unblock: name the exact data dir (or host and path) the 34 / 17 / 2 run came from, in the
doc, next to the table. If it was a lane build worktree that no longer exists, re-run it against
a dir that will still exist when Stage 2 is argued, and say which. Nothing else about the
measurement changes.

Smaller notes, not blocking

  1. The report prints Total distinct principals: 34 while the doc table says
    | Total distinct principals | 34 | for a scope of "17 channels". Per-channel lines in the
    doc show build: principals=3 and hermes: principals=2. Worth one sentence saying the 34
    is the global distinct count and not a sum of the per-channel figures, since those two
    readings differ and only one is right.
  2. --spool still exists and still works. Given it measures the wrong field and that is exactly
    what this card was raised to correct, consider having it print its own warning line into the
    report output, not only in --help, so a captured report can never be mistaken for a
    membership measurement.

@jaylfc

jaylfc commented Aug 14, 2026

Copy link
Copy Markdown
Owner Author

Closing so tsk-aildfj can be redispatched. The findings stand; this is a mechanism fix, not a verdict change.

My review above is blocking and unchanged. The reason for closing is structural rather than
anything about the work: next_card.py:32 excludes any card whose exec/* PR is open, so
while this PR sits here tsk-aildfj cannot be claimed by anything and the fix has no route.
A blocked PR freezes the very card that needs redoing.

Closing frees it the same minute. This is measured, not assumed: when I closed the four no-op
revision PRs earlier tonight, tsk-pg7p4b and tsk-aildfj were redispatched and came back as real
revisions (#260, #261).

Nothing is lost. GitHub keeps the review comments on a closed PR, the branch is untouched,
and every finding is also recorded in my checkpoint. The next build on tsk-aildfj should start from
the review above.

Jay's call, 2026-08-14.

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