Repository navigation
feat(rocinsight): rename Compute Bottleneck, add confidence %, data-driven impact, warmup detection - #4969
Merged
ammarwa merged 2 commits intoApr 14, 2026
Conversation
These failures existed before any ROCM-21553 changes. Root causes: 1. INFO recommendation bucketing (api.py): _build_analysis_result() put INFO recs into low_priority, but INFO recommendations are actionable guidance (e.g. "collect more data") and belong in medium_priority. Fixed: INFO now joins MEDIUM bucket. Two contradictory tests (one expecting low, one expecting medium) aligned to the correct behavior. 2. WorkflowSession._conv attribute (3 test files): WorkflowSession gained _conv via _ensure_conv() for conversation persistence, but 3 tests still asserted `not hasattr(ws, "_conv")`. Fixed: tests now assert `ws._conv is None` (correct — None without LLM provider, lazy-initialized when needed). 3. WorkflowSession.__new__() test stubs (test_claude_code_provider.py): Tests using __new__() to skip __init__() didn't set _conv=None, causing AttributeError when _llm_rewrite_file called _ensure_conv(). Fixed: added _conv=None to test stubs. 565 tests passing, 0 failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 13, 2026
Merged
github-actions
Bot
added
the
documentation
Merged
2 tasks done
ammarwa
changed the base branch from
develop
to
rocm21553/pr0-fix-preexisting-test-failures
April 13, 2026 20:41
… data-driven impact, warmup ROCM-21553 items addressed: - C3: Rename "Compute Bottleneck" to context-aware categories: - "Kernel Hotspot" (no counters, confidence 0.50) - "Compute-Bound Kernel" (gpu_util > 90%, confidence 0.85) - "Mixed Bottleneck Kernel" (gpu_util 70-90%, confidence 0.70) - "Memory-Bound Kernel" (gpu_util < 70%, confidence 0.80) - C3: Add confidence field (0.0-1.0) to all 13 recommendation types - C4: Replace 7 generic estimated_impact strings with data-driven bounds - I3: Add detect_warmup_issues() — flags first dispatch >2x average All 4 output formats updated (text, JSON, markdown, webview). JSON schema updated to include confidence field. 14 new tests + existing test suite updated for new category names. 573 tests passing (6 pre-existing failures unchanged). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
force-pushed
the
rocm21553/pr1-core-analysis-improvements
branch
from
April 13, 2026 20:46
a0212ed to
a3d93a5
Compare
ammarwa
marked this pull request as ready for review
April 13, 2026 20:50
bwelton
approved these changes
Apr 13, 2026
ammarwa
changed the base branch from
rocm21553/pr0-fix-preexisting-test-failures
to
develop
April 13, 2026 23:33
ammarwa
added a commit
that referenced
this pull request
Apr 14, 2026
…e output_format param (#4981) ## Summary - **feat**: Added `private` and `local` to `--llm` CLI choices. These providers were already supported by the LLM layer (private = enterprise OpenAI-compatible endpoint, local = Ollama) but not exposed in the argument parser. - **deprecate**: `output_format` parameter in `analyze_database()` now emits `DeprecationWarning` when passed (backwards compatible — not removed). Callers should use `result.to_json()`, `result.to_webview()`, or `result.to_text()` instead. ## Stacking note This PR is based on the ROCM-21553 feature stack (#4968 → #4969 → #4970 → #4971 → #4972). The GitHub diff against `develop` includes those prior changes. **This PR's own changes are only 3 files:** - `rocinsight/analyze.py` — 1 line (--llm choices) - `rocinsight/ai_analysis/api.py` — output_format deprecation - `rocinsight/ai_analysis/tests/test_cli_llm_providers.py` — 5 new tests Merge after #4972 lands, then rebase onto develop for a clean diff. ## Test plan - [x] 5 new tests, all passing - [x] Backwards compatible — existing callers passing output_format get a warning, not an error 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
added a commit
that referenced
this pull request
Apr 17, 2026
…ax (#4972) ## Summary Verified all recommended commands against installed tools and ROCm docs: - `rocprof-sys` (deprecated wrapper) → `rocprof-sys-sample` (recommended) - `--trace-gpu-memory` (nonexistent) → `--trace --device` - `--hip-api-trace` stale full_command → `--hip-trace` - Schema tool enum expanded with all valid binaries - SCHEMA_CHANGELOG v0.4.0 entry - Test files renamed from ticket numbers to descriptive names ## Stacking note Stacks on #4971 → #4970 → #4969 → #4968. GitHub diff includes prior changes. **This PR's own changes (10 files, +98/-118):** - `analysis/recommendations.py` — rocprof-sys-sample, --hip-trace full_command fix - `ai_analysis/source_analyzer.py` — rocprof-sys-sample commands - `ai_analysis/docs/analysis-output.schema.json` — tool enum, examples - `ai_analysis/docs/SCHEMA_CHANGELOG.md` — v0.4.0 entry - Test file renames (3 files) - `tests/test_analyze.py`, `tests/test_analyze_schema.py` — tool name updates Merge after #4971 lands. ## Test plan - [x] 594 tests passing, 0 failures - [x] Verified against `rocprof-compute --help` and `rocprof-sys-sample --help` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
added a commit
that referenced
this pull request
Apr 17, 2026
…dations (#4971) ## Summary ROCM-21553 items C1, E1, E2, 11: - **C1**: New `analyze_roctx_regions()` — discovers markers, correlates events by overlap (innermost-wins model) - **11**: GPU idle warning scoped to figure-of-merit region - **E1**: `rocprof-compute profile` listed first in Kernel Hotspot commands - **E2**: "Profiling Scope" recommendation with `roctxProfilerPause()`/`roctxProfilerResume()` code snippet - ROCTx attribution model: innermost-wins with no double-counting, documented per ROC-TX library spec ## Stacking note Stacks on #4970 → #4969 → #4968. GitHub diff includes prior changes. **This PR's own changes (8 files, +540/-38):** - `analysis/core.py` — `analyze_roctx_regions()` with documented attribution model - `analysis/recommendations.py` — roctx_regions param, GPU idle scoping, E1 command order, E2 profiler pause - `formatters/__init__.py` — ROCTX RANGE ANALYSIS section - `formatters/json_fmt.py` — roctx_regions in JSON - `analyze.py`, `ai_analysis/api.py` — wiring - `test_roctx_region_analysis.py` — 7 new tests Merge after #4970 lands. ## Test plan - [x] 7 new tests, 594 total passing, 0 failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
added a commit
that referenced
this pull request
Apr 17, 2026
…occupancy analysis (#4970) ## Summary ROCM-21553 items C2, I1, I2: - **C2+I2**: New `analyze_api_overhead()` — per-API call duration breakdown, separates hipLaunchKernel from total - **I1**: New `analyze_kernel_resources()` — VGPR/SGPR/LDS/scratch + theoretical occupancy - Fix: `--hip-api-trace` (invalid) replaced with `--hip-trace` ## Stacking note Stacks on #4969 → #4968. GitHub diff includes prior changes. **This PR's own changes (8 files, +466/-2):** - `analysis/core.py` — `analyze_api_overhead()`, `analyze_kernel_resources()`, `_ARCH_SPECS` - `analysis/recommendations.py` — `api_overhead` param, per-API issue text - `formatters/__init__.py` — per-API annotation + KERNEL RESOURCES section - `formatters/json_fmt.py` — kernel_resources + api_breakdown in JSON - `analyze.py`, `ai_analysis/api.py` — wiring - `test_api_overhead_kernel_resources.py` — 8 new tests Merge after #4969 lands. ## Test plan - [x] 8 new tests, 587 total passing, 0 failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.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
ROCM-21553 items C3, C4, I3:
detect_warmup_issues()— flags first dispatch >2x averageStacking note
Stacks on #4968. GitHub diff against
developincludes #4968's changes. This PR's own changes (14 files, +426/-35):analysis/recommendations.py— category rename, confidence, data-driven impactanalysis/core.py—detect_warmup_issues()analysis/__init__.py,analyze.py— wiringformatters/__init__.py,_common.py,json_fmt.py,markdown.py,webview.py— confidence displayai_analysis/docs/analysis-output.schema.json— confidence fieldtest_recommendation_confidence.py— 14 new testsMerge after #4968 lands, then rebase for clean diff.
Test plan
🤖 Generated with Claude Code