Skip to content

Re-do the identity-stem measurement honestly: senders not membership, with a minimum-evidence guard (replaces defective tsk-aildfj) - #274

Closed
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-knncne
Closed

Re-do the identity-stem measurement honestly: senders not membership, with a minimum-evidence guard (replaces defective tsk-aildfj)#274
jaylfc wants to merge 1 commit into
masterfrom
exec/tsk-knncne

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 14, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Re-do the identity-stem measurement honestly: senders not membership, with a minimum-evidence guard (replaces defective tsk-aildfj)

Autonomous build of board card tsk-knncne.

  • Measure distinct from-values per channel, not membership
  • Make --data-dir REQUIRED (remove bus-spool.jsonl fallback)
  • Add minimum-evidence guard: INSUFFICIENT DATA when principal count < 3,
    SAFE must never be reachable from empty/near-empty input
  • Remove redundant collapse_without_mint (identical to multi_spell_stems_without_mint)
  • Update doc tsk-rf5gwb with real data numbers; CONCLUSION preserved: NOT safe
  • Add positive/negative test controls

Files:
docs/specs/tsk-rf5gwb-membership-stems.md | 84 ++++++++++
scripts/measure_channel_sender_stems.py | 201 +++++++++++++++++++++++
tests/test_measure_channel_sender_stems.py | 255 +++++++++++++++++++++++++++++
3 files changed, 540 insertions(+)

…dence guard

- Measure distinct from-values per channel, not membership
- Make --data-dir REQUIRED (remove bus-spool.jsonl fallback)
- Add minimum-evidence guard: INSUFFICIENT DATA when principal count < 3,
  SAFE must never be reachable from empty/near-empty input
- Remove redundant collapse_without_mint (identical to multi_spell_stems_without_mint)
- Update doc tsk-rf5gwb with real data numbers; CONCLUSION preserved: NOT safe
- Add positive/negative test controls
@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 14, 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: 11 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: 174024cd-c4d2-4c90-906e-4fe8dcb9f90c

📥 Commits

Reviewing files that changed from the base of the PR and between 1019775 and b3744fc.

📒 Files selected for processing (3)
  • docs/specs/tsk-rf5gwb-membership-stems.md
  • scripts/measure_channel_sender_stems.py
  • tests/test_measure_channel_sender_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 14, 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

Reviewed by running it. Most of the card is delivered, and two things that go to the heart of it are not. Both are small edits, not a rebuild.

Delivered, verified

  • --data-dir is required=True. The silent spool fallback that made the default invocation measure the forbidden source is gone.
  • The INSUFFICIENT DATA guard is real at the CLI, and it has a genuine negative control. test_insufficient_data_with_few_principals seeds a single sender, runs the script as a subprocess, and asserts returncode != 0 and "INSUFFICIENT DATA" in stderr. That test goes red if the guard is removed, which is exactly what the card asked for and what Revise PR #250: measure REAL channel membership, not spool senders (Stage 2 entry gate) #268 never had.
  • The analysis core still works. Positive control with @a / a / a-20260726-171307 trips NOT safe.
  • Renamed to measure_channel_sender_stems.py, argparse description "Measure distinct from-values per channel".

REQUIRED 1 - the conclusion still claims to measure membership

The rename reached the filename and the argparse help. It did not reach the sentence a human actually reads:

CONCLUSION [scope: ...]: mint-stamp stripping is safe for membership.
2. Canonical membership entries with bare or @-form twin: 0

Measured: the word membership still appears twice in print_report output.

This is the substantive point of the card, not a naming nit. There is no channel membership in this system - a2a_channels builds members from data["from"], a2a_members is documented as "distinct sender names observed", and no roster table exists. And it now contradicts the spec directly: docs/specs/a2a-bus-auth-transition.md (commit 1019775) states that Stage 2 may key on sender-derived identity but may not describe it as membership. If this merges as written, the gate document and the tool that feeds it disagree in their own words.

Say what is measured: distinct from values per channel. mint-stamp stripping is safe for the sender-stem grouping, Canonical sender entries with bare or @-form twin.

REQUIRED 2 - print_report is still fails-open, the guard just moved outside it

The guard lives in async_main. print_report is unchanged, so the exact surface this card was written about still does this:

>>> print_report(measure([]), "SYNTHETIC empty")
CONCLUSION [scope: SYNTHETIC empty]: mint-stamp stripping is safe for membership.
   prints "safe": True        prints "INSUFFICIENT": False

Zero data still returns the SAFE verdict from the function that emits the verdict. Today the CLI is the only caller, so the tool as invoked is safe - which is why this is a required change and not a blocker. But the reason it matters is the reason the card exists: the fails-open behaviour was never a CLI bug, it was that a conclusion was being drawn from absence of counterexamples. Any test, import, or future wrapper that calls print_report gets it straight back, and it will look correct.

Move the check into print_report (or have it take an explicit observed-volume argument and refuse below it). The CLI guard can stay; two layers is right for this one. Then add the function-level negative control alongside the CLI one: print_report(measure([]), ...) must not contain "safe".

One smaller thing

The threshold is a bare < 3 distinct principals, hardcoded and not stated in the output. Three principals is very little for "enough was observed that a counterexample could have appeared". Print the observed volume next to the verdict so a reader can judge it, and put the number in the doc.

Not blocking, and not asking for a rebuild - the structure is right and the CLI negative control is the hard part, already done.

@jaylfc

jaylfc commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Closing in favour of the revision card tsk-hjm3vg, which carries the two required changes from my review above verbatim.

Nothing about that review has changed: the head is still b3744fc, and both points stand. I am closing rather than leaving this open because a blocked PR has no revision path in this fleet, and closing frees the throttle slot and clears the one-PR-per-task guard so the follow-up can open cleanly. This is the same route #260 took to a merged #299.

tsk-hjm3vg branches from exec/tsk-knncne, so everything already delivered survives untouched, including the CLI negative control, which was the hard part. The branch is deliberately NOT deleted. Please leave it in place.

One addition to the acceptance criteria, learned from #255 today: the suite has to be run against a trial merge with current master, not just against the branch. #255 was approved on 2026-08-14 with five green checks and its own test failed the moment current master was merged in, because master had drifted underneath it.

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