feat(sccm): preserve client intake capture gaps - #436
Conversation
📝 WalkthroughWalkthroughSCCM client intake now supports coverage-only capture gaps. Bundles and assessments serialize, validate, group, sort, and aggregate these declarations. Tests cover wire compatibility, shared limits, collision detection, provenance, and deterministic ordering. ChangesCapture-gap intake support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SccmClientIntakeBundle
participant IntakeWireFormat
participant SccmClientIntakeAssessment
participant CoverageAggregation
SccmClientIntakeBundle->>IntakeWireFormat: serialize captureGaps
IntakeWireFormat->>SccmClientIntakeBundle: bounded deserialization and validation
SccmClientIntakeBundle->>SccmClientIntakeAssessment: construct canonical assessment
SccmClientIntakeAssessment->>CoverageAggregation: group and aggregate capture gaps
CoverageAggregation-->>SccmClientIntakeAssessment: sorted coverage-gap entries
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
github-actions
Bot
added
enhancement
|
@coderabbitai review Please review exact head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR extends the SCCM client intake contract to represent coverage-only omissions (capture gaps) for recognized client source rotations (specifically Capped and ParseFailed) without changing physical artifacts or public LogEntry behavior. It adds a new projection type and ensures wire compatibility (empty additive field omitted) while enforcing a single shared declaration quota across artifacts and capture gaps.
Changes:
- Introduces
SccmClientIntakeCaptureGapplus bundle/assessment support for additivecaptureGapswith shape validation. - Enforces a shared, order-independent
MAX_SCCM_CLIENT_INTAKE_ARTIFACTSquota acrossartifacts + captureGapsduring bundle deserialization and bundle validation. - Expands the test suite to cover capture-gap semantics, deterministic projection/ordering, compatibility (empty field omission), and quota/malformed precedence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/cmtraceopen-parser/src/sccm/client/intake.rs | Adds capture-gap type + serde/validation, bundle/assessment wire support, shared quota enforcement, and coverage projection updates. |
| crates/cmtraceopen-parser/tests/sccm_client_intake.rs | Adds/updates tests for capture-gap behavior, serde round-trips, compatibility guarantees, and shared-quota boundary cases. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/cmtraceopen-parser/src/sccm/client/intake.rs (1)
357-464: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider one generic bounded-sequence seed instead of two copies.
BoundedCaptureGapsSeed/BoundedCaptureGapVisitorandBoundedArtifactsSeed/BoundedArtifactVisitorare identical apart from the element type and theexpectingwording. A single generic seed keeps the quota semantics in one place, so a future fix to thesize_hintpre-check or theIgnoredAnyprobe cannot land in only one copy.The current logic is correct, including the exact-boundary case and the
limit == 0case.♻️ Sketch of a generic bounded seed
struct BoundedSeqSeed<T> { limit: usize, label: &'static str, element: PhantomData<T>, } impl<'de, T: Deserialize<'de>> DeserializeSeed<'de> for BoundedSeqSeed<T> { type Value = Vec<T>; fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error> where D: Deserializer<'de>, { deserializer.deserialize_seq(BoundedSeqVisitor { limit: self.limit, label: self.label, element: PhantomData, }) } }Call sites then read
BoundedSeqSeed::<SccmClientIntakeCaptureGap>::new(remaining, "SCCM client capture gaps").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cmtraceopen-parser/src/sccm/client/intake.rs` around lines 357 - 464, Replace the duplicated BoundedCaptureGapsSeed/BoundedCaptureGapVisitor and BoundedArtifactsSeed/BoundedArtifactVisitor implementations with one generic BoundedSeqSeed and visitor parameterized by the element type and description label. Reuse the shared size_hint check, bounded collection, and IgnoredAny overflow probe, while preserving the existing exact-boundary and limit == 0 behavior; update both call sites to pass the appropriate element type, remaining limit, and expecting label.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/cmtraceopen-parser/src/sccm/client/intake.rs`:
- Around line 357-464: Replace the duplicated
BoundedCaptureGapsSeed/BoundedCaptureGapVisitor and
BoundedArtifactsSeed/BoundedArtifactVisitor implementations with one generic
BoundedSeqSeed and visitor parameterized by the element type and description
label. Reuse the shared size_hint check, bounded collection, and IgnoredAny
overflow probe, while preserving the existing exact-boundary and limit == 0
behavior; update both call sites to pass the appropriate element type, remaining
limit, and expecting label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 562b304f-5a19-4ef4-89b0-f532178f5fa0
📒 Files selected for processing (2)
crates/cmtraceopen-parser/src/sccm/client/intake.rscrates/cmtraceopen-parser/tests/sccm_client_intake.rs
Scope
Advances #319 with the additive pure-Rust SCCM client capture-gap contract required before the native adapter can represent omitted rotations without relabeling retained evidence.
SccmClientIntakeCaptureGapprojection forCappedandParseFailedomissions;LogEntrybehavior unchanged;No Windows I/O, registry, WMI, Tauri, network, database, workflow reducer, native capture, or live-Windows claim is included. Raw CCM remains the shared grammar; no parser kind was added.
Test-first review history
RED cases reproduced during review/correction:
The final exact head
fd52c4ceb6bde211f26d3647eda010e8fe81d8c0passed a separate detached-worktree adversarial review across standalone/bundle/assessment serde, field order, malformed-over-quota precedence, collision matrices, deterministic projection, and compatibility behavior.Verification
git diff --check, andgit show --check: passed;Advisory retained for follow-up: malformed unknown capture-gap field/enum names can appear in Serde error strings. The top-level bundle behavior is inherited and no current non-test deserializer/IPC exporter was found, so this is not represented as evidence output or a live privacy failure. Re-evaluate before any caller exposes raw parser errors through IPC/UI.
Dependency and next merge order
Target is
codex/parser-family-skeletonatce774fd60c2d6e209a4bf69ea968865bf0d205f7. Native client work remains local/NO-GO and must restack on the resulting merge commit, map native gaps explicitly, enforce the same global quota, sanitize legacy errors, and pass fresh review plus Windows acceptance. Keep #319 open.Summary by CodeRabbit
New Features
Bug Fixes