Fix #28 (Issue 1): Suppress script output in JSON mode for clean parsing #31
+33
−4
Conversation
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
Resolves Issue #28 (Issue 1) by making --json output parseable with jq and other JSON tools. Problem: - Script stdout/stderr mixed with JSON output - Broke jq parsing: `./bin/xdebug-profile --json -- php script.php | jq` - Prevented piping to other JSON processing tools - Common error: "parse error: Invalid numeric literal" Solution: - In JSON mode, redirect script output to /dev/null - Keep exit code for error detection - Use passthru() for human mode (show script output) - Use exec() for JSON mode (suppress script output) Benefits: - ✅ Clean JSON output for jq: `... | jq '.["🎯 bottleneck_functions"]'` - ✅ MCP-compatible: pure JSON response - ✅ Tool-friendly: easy to integrate with other tools - ✅ Backward compatible: human mode unchanged Changes: - bin/xdebug-profile: Conditional output suppression in JSON mode - docs/TROUBLESHOOTING.md: Added JSON parsing issue and solution - Updated --help text with clear behavior explanation - Added jq usage example to help text 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
レビュー担当者向けガイド（小規模PRでは折りたたみ表示）レビュー担当者向けガイドこのPRは、 JSONモードでの条件付き出力抑制のシーケンス図sequenceDiagram
participant User as actor User
participant CLI as "xdebug-profile CLI"
participant Script as "Target PHP Script"
User->>CLI: Run xdebug-profile with --json flag
CLI->>Script: Execute script (output suppressed)
Script-->>CLI: Script runs (stdout/stderr redirected)
CLI-->>User: Return pure JSON output
User->>CLI: Run xdebug-profile without --json
CLI->>Script: Execute script (output visible)
Script-->>CLI: Script runs (stdout/stderr shown)
CLI-->>User: Return human-readable output with script output
更新された出力処理ロジックのクラス図classDiagram
class XdebugProfileCLI {
+bool jsonOutput
+int exitCode
+runScript(cmd)
}
XdebugProfileCLI : +runScript(cmd)
XdebugProfileCLI : if jsonOutput
XdebugProfileCLI : cmd += ' > /dev/null 2>&1'
XdebugProfileCLI : exec(cmd, output, exitCode)
XdebugProfileCLI : else
XdebugProfileCLI : passthru(cmd, exitCode)
XdebugProfileCLI : # Output handling logic updated
ファイルレベルの変更点
関連する可能性のある課題
ヒントとコマンドSourceryとの連携
エクスペリエンスのカスタマイズダッシュボードにアクセスして、以下を行うことができます：
ヘルプ
Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR introduces conditional suppression of script stdout/stderr when the --json flag is used to ensure pure, parseable JSON output, and updates the CLI help text and troubleshooting documentation with examples for jq integration. Sequence diagram for conditional output suppression in JSON modesequenceDiagram
participant User as actor User
participant CLI as "xdebug-profile CLI"
participant Script as "Target PHP Script"
User->>CLI: Run xdebug-profile with --json flag
CLI->>Script: Execute script (output suppressed)
Script-->>CLI: Script runs (stdout/stderr redirected)
CLI-->>User: Return pure JSON output
User->>CLI: Run xdebug-profile without --json
CLI->>Script: Execute script (output visible)
Script-->>CLI: Script runs (stdout/stderr shown)
CLI-->>User: Return human-readable output with script output
Class diagram for updated output handling logicclassDiagram
class XdebugProfileCLI {
+bool jsonOutput
+int exitCode
+runScript(cmd)
}
XdebugProfileCLI : +runScript(cmd)
XdebugProfileCLI : if jsonOutput
XdebugProfileCLI : cmd += ' > /dev/null 2>&1'
XdebugProfileCLI : exec(cmd, output, exitCode)
XdebugProfileCLI : else
XdebugProfileCLI : passthru(cmd, exitCode)
XdebugProfileCLI : # Output handling logic updated
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
こんにちは - あなたの変更を確認しました - いくつかフィードバックがあります：
- 出力抑制のために
/dev/nullをハードコーディングすると、Windows で動作しない可能性があります。PHP の出力バッファリングのようなクロスプラットフォームなアプローチを使用するか、適切なヌルデバイスを検出することを検討してください。 - JSON モードですべてのスクリプト出力をサイレントに破棄すると、デバッグが難しくなる可能性があります。オプションで標準エラー出力をキャプチャし、JSON フィールドに含めるか、ログファイルにリダイレクトすることを検討してください。
Prompt for AI Agents
このコードレビューのコメントに対処してください：
## 全体的なコメント
- 出力抑制のために `/dev/null` をハードコーディングすると、Windows で動作しない可能性があります。PHP の出力バッファリングのようなクロスプラットフォームなアプローチを使用するか、適切なヌルデバイスを検出することを検討してください。
- JSON モードですべてのスクリプト出力をサイレントに破棄すると、デバッグが難しくなる可能性があります。オプションで標準エラー出力をキャプチャし、JSON フィールドに含めるか、ログファイルにリダイレクトすることを検討してください。Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Hardcoding /dev/null for output suppression may break on Windows; consider using a cross-platform approach like PHP’s output buffering or detecting the appropriate null device.
- Silently discarding all script output in JSON mode can make debugging harder; consider optionally capturing stderr and including it in a JSON field or redirecting it to a log file.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Hardcoding /dev/null for output suppression may break on Windows; consider using a cross-platform approach like PHP’s output buffering or detecting the appropriate null device.
- Silently discarding all script output in JSON mode can make debugging harder; consider optionally capturing stderr and including it in a JSON field or redirecting it to a log file.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add entry for Issue #28 (Issue 1) in Unreleased section. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/TROUBLESHOOTING.md (1)
220-220: Replace version placeholder with actual version.The comment references "v0.x.x" which appears to be a placeholder. Consider replacing this with the actual version where this fix was introduced or removing the version reference entirely if it's not critical.
Apply this diff if you want to remove the version reference:
-# ❌ Before v0.x.x: Script output breaks jq parsing +# ❌ Before: Script output breaks jq parsing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bin/xdebug-profile(4 hunks)docs/TROUBLESHOOTING.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-02T16:54:01.944Z
Learnt from: CR
Repo: koriym/xdebug-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T16:54:01.944Z
Learning: Default tool selection: use ./bin/xdebug-profile for general analysis, ./bin/xdebug-trace for execution flow, ./bin/xdebug-coverage for coverage, ./bin/xdebug-debug for interactive debugging
Applied to files:
bin/xdebug-profiledocs/TROUBLESHOOTING.md
📚 Learning: 2025-09-02T16:54:01.944Z
Learnt from: CR
Repo: koriym/xdebug-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T16:54:01.944Z
Learning: Prioritize runtime execution data (trace/profile/coverage) over static code analysis when assisting with PHP debugging
Applied to files:
bin/xdebug-profile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
docs/TROUBLESHOOTING.md (1)
233-234: LGTM!The jq filtering examples clearly demonstrate the clean JSON output functionality. These examples will help users understand how to leverage the fixed JSON output with standard tooling.
bin/xdebug-profile (3)
31-31: LGTM!The help text updates clearly communicate the JSON mode behavior and provide a practical jq filtering example. The documentation accurately reflects the implementation changes.
Also applies to: 54-56, 66-67
167-174: Consider documenting error debugging workflow for JSON mode.In JSON mode, all script output (including errors, warnings, and exceptions) is suppressed for clean JSON parsing. While this achieves the PR objective, it may hinder debugging when the profiled script fails. Users will see a non-zero exit code but no error messages.
Consider adding a note in the help text or troubleshooting guide suggesting that users should:
- Run without
--jsonfirst to verify the script works- Check exit codes in JSON mode to detect failures
- Re-run without
--jsonif debugging is neededExample addition to help text:
CORE OPTIONS: - --json AI-optimized JSON output (suppresses script output for clean parsing) + --json AI-optimized JSON output (suppresses script output for clean parsing) + Note: Errors are suppressed; check exit code or run without --json to debugIs this trade-off acceptable, or should critical errors (e.g., PHP fatal errors) remain visible even in JSON mode?
166-166: LGTM!Exit code handling is correct. The script properly captures the exit code from both exec() and passthru() calls and propagates it at the end, ensuring that failures in the profiled script are detectable even when output is suppressed.
Also applies to: 520-520
Comment on lines
+167
to
+174
| if ($jsonOutput) { | ||
| // JSON mode: suppress script output for clean JSON parsing | ||
| $cmd .= ' > /dev/null 2>&1'; | ||
| exec($cmd, $output, $exitCode); | ||
| } else { | ||
| // Human-readable mode: show script output | ||
| passthru($cmd, $exitCode); | ||
| } |
There was a problem hiding this comment.
Critical: Windows compatibility issue with /dev/null redirection.
The redirection > /dev/null 2>&1 is Unix/Linux specific and will fail on Windows systems. Windows uses NUL instead of /dev/null.
Apply this diff to add cross-platform support:
+// Determine null device based on OS
+$nullDevice = PHP_OS_FAMILY === 'Windows' ? 'NUL' : '/dev/null';
+
// Execute command
$exitCode = 0;
if ($jsonOutput) {
// JSON mode: suppress script output for clean JSON parsing
- $cmd .= ' > /dev/null 2>&1';
+ $cmd .= " > {$nullDevice} 2>&1";
exec($cmd, $output, $exitCode);
} else {
// Human-readable mode: show script output
passthru($cmd, $exitCode);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bin/xdebug-profile around lines 167 to 174, the current redirection uses the
Unix-specific '/dev/null' which breaks on Windows; detect the platform (for
example via PHP_OS_FAMILY or checking PHP_OS for 'WIN') and set a $nullDevice
variable to 'NUL' on Windows and '/dev/null' otherwise, then append the
redirection using that variable (e.g., " > {nullDevice} 2>&1") when building
$cmd for JSON mode so exec() works cross-platform; leave the passthru path
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
57-61: Well-written changelog entry; consider clarifying the issue scope.The entry accurately captures the fix and includes a practical jq example. However, since Issue #28 has multiple parts (as noted in the PR objectives), consider clarifying that this entry addresses "Issue 1 from #28" to distinguish it from the earlier %p placeholder fix on lines 53–56, which also references #28.
Suggested revision:
- **JSON Output Parsing**: Fixed `xdebug-profile --json` to suppress script output for clean JSON ([#28](https://github.com/koriym/xdebug-mcp/issues/28)) + **JSON Output Parsing**: Fixed `xdebug-profile --json` to suppress script output for clean JSON ([#28](https://github.com/koriym/xdebug-mcp/issues/28) - Part 1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T16:54:01.944Z
Learnt from: CR
Repo: koriym/xdebug-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T16:54:01.944Z
Learning: Default tool selection: use ./bin/xdebug-profile for general analysis, ./bin/xdebug-trace for execution flow, ./bin/xdebug-coverage for coverage, ./bin/xdebug-debug for interactive debugging
Applied to files:
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
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
Resolves Issue #28 (Issue 1) by making
--jsonoutput clean and parseable withjqand other JSON tools.Problem
When using
--jsonmode, script stdout/stderr mixed with JSON output:Root Cause:
jqparsing and piping to other toolsSolution
Automatically suppress script output when
--jsonis used:Benefits
✅ Clean JSON output - No more mixed output:
✅ jq compatible - Pipe directly to jq for filtering
✅ MCP compatible - Pure JSON response for AI integration
✅ Tool-friendly - Easy integration with other JSON processors
✅ Backward compatible - Human-readable mode unchanged
Changes
Code:
bin/xdebug-profile: Conditional output suppression based on$jsonOutputflagDocumentation:
bin/xdebug-profile --help: Updated option descriptions and added jq exampledocs/TROUBLESHOOTING.md: Added section on JSON output issues with before/after examplesUsage Examples
Before (broken):
./bin/xdebug-profile --json -- php script.php # Mixed output breaks parsingAfter (clean):
Testing
Verified that:
Related
This addresses Issue 1 from #28. Other enhancements (--top, --filter) will be addressed in separate PRs.
🤖 Generated with Claude Code
Sourceryによる要約
JSONモードでのスクリプト出力を抑制し、クリーンでパース可能なJSONを生成し、jqフィルタリングの使用例を含むドキュメントを更新
バグ修正:
--json使用時にスクリプトの標準出力および標準エラー出力を抑制し、出力の混在を防ぎ、クリーンなJSONパースを可能にする機能改善:
ドキュメント:
Original summary in English
Summary by Sourcery
Suppress script output in JSON mode to produce clean, parseable JSON and update documentation with usage examples for jq filtering
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Documentation