Skip to content

fix(besreceiver): correct PatternExpanded semantics and surface pattern_skipped - #75

Merged
chagui merged 1 commit into
mainfrom
chagui/besreceiver/issue-62-followups
Apr 25, 2026
Merged

chagui merged 1 commit into
mainfrom
chagui/besreceiver/issue-62-followups

Conversation

@chagui

@chagui chagui commented Apr 25, 2026

Copy link
Copy Markdown
Owner

Summary

Three P0 corrections to bazel.patterns from the post-merge review of #71.

  • target_count now filters to TargetConfigured children. Previously len(children) counted nested PatternExpanded ids and UnconfiguredLabel failure markers, inflating the reported count of resolved targets.
  • Multi-pattern events even-split target_count across the event's non-empty patterns (floor(M/N)). The proto carries no per-pattern attribution; the prior code stamped the full count to each pattern, summing to N×M across the slice. Single-pattern events — Bazel's dominant case — keep the exact count. Eliminates the inflation that the prior multi-pattern test had cemented as the contract.
  • pattern_skipped events surface as a new bazel.patterns_skipped Slice[string] on the root span instead of being silently dropped. Bazel emits these under --keep_going for parts of a top-level pattern whose expansion was skipped. eventTypeName distinguishes them so bes.events.processed no longer conflates the two id shapes.

docs/attributes.md drops the misleading "1,400 tests" example and documents the new even-split + filter semantics plus the new attribute.

Why this approach

Multi-pattern events are rare (Bazel emits one PatternExpanded per top-level pattern in practice), and the proto offers no per-pattern attribution within them. Even-split keeps the slice shape stable, makes the per-event sum match the child count, and gives an exact answer for the common case. Reusing caps.Patterns for bazel.patterns_skipped keeps operator tuning to one knob.

Test plan

  • make build
  • make vet
  • make test — 360 tests, 4 new + 1 renamed/updated
    • TestPatternExpanded_NonTargetConfiguredChildren_NotCounted
    • TestPatternExpanded_PatternSkipped_EmitsSkippedAttribute
    • TestPatternExpanded_PatternSkippedOnly_NoPatternsAttribute
    • TestPatternExpanded_PostFlush_Dropped
    • TestPatternExpanded_MultiplePatterns_EvenSplitsTargetCount (renamed, semantics inverted from prior assertion)
  • make lint

Refs #62, follow-up to #71.

🤖 Generated with Claude Code

…rn_skipped

Three P0 corrections to bazel.patterns from the post-merge review of #71:

1. target_count filters to TargetConfigured children. Previously len(children)
   counted nested PatternExpanded ids and UnconfiguredLabel failure markers,
   inflating the reported count of resolved targets.

2. Multi-pattern events even-split target_count across the event's non-empty
   patterns (floor(M/N)). The proto carries no per-pattern attribution; the
   prior code stamped the full count to each pattern, summing to N×M across
   the slice. Single-pattern events — Bazel's dominant case — keep the exact
   count. Eliminates the inflation that the prior multi-pattern test had
   cemented as the contract.

3. pattern_skipped events route to a new bazel.patterns_skipped Slice[string]
   on the root span, instead of being silently dropped. Bazel emits these
   under --keep_going for parts of a top-level pattern whose expansion was
   skipped. eventTypeName surfaces them under a distinct event_type label
   so bes.events.processed counts the two id shapes separately.

Test coverage added: NonTargetConfiguredChildren_NotCounted,
PatternSkipped_EmitsSkippedAttribute, PatternSkippedOnly_NoPatternsAttribute,
PostFlush_Dropped. The existing multi-pattern test is renamed and updated to
assert the corrected even-split semantics. docs/attributes.md drops the
"1,400 tests" example and documents the new semantics + the new attribute.

Refs #62, follow-up to #71.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@chagui
chagui merged commit db015b8 into main Apr 25, 2026
5 checks passed
@chagui
chagui deleted the chagui/besreceiver/issue-62-followups branch April 25, 2026 19:38
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