Repository navigation
Qualify SampleProfiler CPU evidence and preserve loss warnings - #166
Merged
Merged
Conversation
Distinguish SampleProfiler thread-stack counts from established ETW CPU time, qualify CPU-facing guidance, and retain capture-loss warnings when manifest case outputs are bounded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The warning budget can hide case-specific failures, and changed cross-provider CPU semantics warrant human review.
Review effort: Balanced
Findings: 1
What changed in this PR
This PR makes CPU results distinguish SampleProfiler thread-stack counts from on-core CPU time across the analyzer, CLI, and MCP server.
Changes:
- Track ETW and SampleProfiler evidence and qualify CPU warnings and follow-up hints.
- Preserve lost-event and provider warnings in bounded manifest batch and diff results.
- Add regression tests and update workflow guidance.
| File | Description |
|---|---|
| tests/Filtrace.Mcp.Tests/TraceToolsTests.cs | Tests MCP warnings and hints. |
| tests/Filtrace.Core.Tests/TraceLoaderTests.cs | Tests loaded sample provenance. |
| tests/Filtrace.Core.Tests/TimelineProviderTests.cs | Tests timeline provider warnings. |
| tests/Filtrace.Core.Tests/SteeringHintsTests.cs | Tests qualified follow-up hints. |
| tests/Filtrace.Core.Tests/ProcessInventoryProviderTests.cs | Tests process inventory provenance. |
| tests/Filtrace.Core.Tests/CpuSampleEvidenceTests.cs | Tests provider-specific weighting. |
| tests/Filtrace.Core.Tests/CaptureManifestReaderTests.cs | Tests bounded manifest warnings. |
| tests/Filtrace.Core.Tests/ActivityScopeTests.cs | Tests warnings after activity filtering. |
| tests/Filtrace.Cli.Tests/RankingExecutorTests.cs | Tests ranking output. |
| tests/Filtrace.Cli.Tests/DiffExecutorTests.cs | Tests diff warning labels. |
| tests/Filtrace.Cli.Tests/CliAppTests.cs | Tests CLI JSON results. |
| src/Filtrace/Cli/TimelineExecutor.cs | Emits timeline CPU warnings. |
| src/Filtrace/Cli/RankingExecutor.cs | Passes provenance to ranking hints. |
| src/Filtrace.Mcp/TraceTools.cs | Carries provenance and timeline warnings through MCP. |
| src/Filtrace.Core/Tracing/TraceLoader.cs | Retains lost-event counts. |
| src/Filtrace.Core/Tracing/TraceInfo.cs | Stores lost-event counts. |
| src/Filtrace.Core/Tracing/Readers/TraceReadResult.cs | Carries reader loss counts. |
| src/Filtrace.Core/Tracing/Readers/TraceLogReader.cs | Classifies and weights CPU samples. |
| src/Filtrace.Core/Tracing/Readers/CpuSampleEvidence.cs | Defines sample evidence and warnings. |
| src/Filtrace.Core/Tracing/Providers/TimelineResult.cs | Holds timeline CPU warnings. |
| src/Filtrace.Core/Tracing/Providers/TimelineProvider.Snapshot.cs | Detects snapshot sample providers. |
| src/Filtrace.Core/Tracing/Providers/TimelineProvider.EventLanes.cs | Carries event-lane warnings. |
| src/Filtrace.Core/Tracing/Providers/TimelineProvider.cs | Detects CPU-lane sample providers. |
| src/Filtrace.Core/Tracing/Providers/ProcessInventoryProvider.cs | Applies sample provenance to inventory. |
| src/Filtrace.Core/Tracing/CpuSampleProvenance.cs | Clarifies provenance documentation. |
| src/Filtrace.Core/Tracing/CaptureManifestOutput.cs | Prioritizes bounded warnings. |
| src/Filtrace.Core/Tracing/CaptureManifestDiffAnalyzer.cs | Reserves diff-arm warnings. |
| src/Filtrace.Core/Tracing/CaptureManifestBatchAnalyzer.cs | Reserves batch warnings. |
| src/Filtrace.Core/Output/SteeringHints.cs | Qualifies CPU follow-up language. |
| docs/workflow.md | Documents sample-count interpretation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Accept schema 18 SampleProfiler and mixed sample-count evidence in the Track D validator without admitting false time or malformed interval claims. Keep capture loss and sample provenance while reserving bounded manifest case warnings for root and per-operation diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Mixed-provider CPU interpretation changes several analysis paths, but combined real-capture behavior has only synthetic coverage.
Review effort: Balanced
Findings: 1
Open (1)
Resolved since last review (1)
Pass the schema version through count conversion and provenance validation so malformed schema 18 results no longer report schema 17. Preserve accepted inputs and pin missing, malformed, and legacy errors in the Track D contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Mixed-provider weighting has synthetic coverage; actual ETW and EventPipe fixtures were checked separately. No machine-wide capture is included.