Add --watch option to xstep

[koriym]

Summary

• Add --watch=EXPR option to xstep that evaluates PHP expressions at each step and only records steps where the watched value changes
• Dramatically reduces token consumption when debugging loops (e.g., 1000 loop iterations -> only steps where watched variable actually changes)
• Supports multiple --watch flags, tracks initial/changed/out_of_scope transitions with previous values

Changes

• bin/xstep: Parse --watch=EXPR arguments (multiple allowed)
• src/DebugServer.php: Add evaluateWatchExpression() method and watch-based filtering in performStepTrace()
• docs/schemas/xstep.json: Add watches field to breakpoint schema definition
• tests/Unit/DebugServerTest.php: Add tests for watches option and method existence

Test plan

• composer tests passes (PHPCS, PHPStan, PHPUnit)
• composer pcov coverage 98.80%
• xcoverage 100% (416/416 lines)
• CI passes
• CodeRabbit review

Summary by CodeRabbit

• New Features

  • Added --watch=EXPR to monitor expressions during step-trace. Steps now include per-watch data (current, previous, reason: initial/changed/out_of_scope) and recordings are filtered to emit steps only when watched values change.

• Documentation

  • CLI help updated to document the multi-line --watch option and its behavior.

• Tests

  • Added unit tests validating watches handling and evaluation behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a global --watch=EXPR flag to xstep; collects multiple watches, propagates them to DebugServer, evaluates watch expressions each step, attaches per-step watch metadata (expression, value, previous, reason), and when watches are present only records steps where at least one watched value changed.

Changes

Cohort / File(s)	Summary
CLI Option Parsing bin/xstep	Added parsing for multiple --watch=EXPR flags into a watches array; updated help text and passed watches into both advanced breakpoint and legacy DebugServer initialization paths.
Schema Updates docs/schemas/xstep.json	Added watches array to breakpoint schema; watch items include expression, value, previous (nullable), and reason (enum: initial, changed, out_of_scope) with expression, value, and reason required.
Debug Server Logic src/DebugServer.php	Implemented per-step watch evaluation and recording: added watch handling (tracking previous values, reasons), evaluateWatchExpression() logic, content-hash helper, guarded single output emission, attach watches metadata to steps, and skip recording steps when no watch changed. Docblock updated to document watches option.
Unit Tests tests/Unit/DebugServerTest.php	Added tests for constructor handling of watches (non-empty and empty) and presence/signature check for private evaluateWatchExpression() method.

Sequence Diagram

sequenceDiagram
    participant User as User/CLI
    participant Parser as bin/xstep Parser
    participant Server as DebugServer
    participant DBGp as DBGp Evaluator
    participant Recorder as Step Recorder

    User->>Parser: Run xstep with --watch=EXPR...
    Parser->>Parser: Collect watches[] and other options
    Parser->>Server: Start DebugServer with watches config

    loop for each program step
        Server->>DBGp: Evaluate each watch expression
        DBGp-->>Server: Return value or <unavailable>
        Server->>Server: Compute current watch data (expression,value,previous,reason)
        Server->>Server: Compare with previous values

        alt any watched value changed
            Server->>Recorder: Record step + watches metadata
            Recorder-->>Server: Acknowledge recording
        else no watched changes
            Server->>Server: Skip recording this step
        end

        Server->>Server: Update previous watch values
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• feat: Add --watch option to xstep for expression change tracking #47 — Implements the same --watch support (CLI parsing, propagation to DebugServer, per-step evaluation/filtering, schema and tests), likely the same feature request.

Poem

🐰 I nibble code and watch each change,
Little expressions hop and range,
Steps recorded when values shift,
A tiny trace, a debugging gift,
Hooray — the watchful trail grows strange!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add --watch option to xstep' clearly and concisely summarizes the main feature being added, which is corroborated by changes across multiple files implementing watch expression support.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-watch-option

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[koriym]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/DebugServer.php`:
- Around line 1035-1089: evaluateWatchExpression currently summarizes
arrays/objects as "array(n)" or "object: Class", which prevents watches from
detecting internal mutations; update evaluateWatchExpression to, when $type is
'array' or 'object', attempt to include a content-derived signature (e.g., hash
of serialize() or json_encode() of child properties) instead of only
count/class: inspect $prop children (or use $this->propertyGet/children data
returned by parseXmlResponse), build a stable string representation of child
values (falling back to the existing summary if children are unavailable), then
return a short hash or encoded snippet so watches change when internal contents
change; keep the existing fallback behavior for missing data.

🧹 Nitpick comments (1)

src/DebugServer.php (1)

637-667: Consider separating executed-step cap from recorded-step cap when watch filtering is enabled.
$stepCount increments on skipped steps, so long loops with no watch changes can hit maxSteps before any recorded change. A separate executedSteps safety cap plus recordedSteps for maxSteps would preserve safety while keeping the watch feature useful.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/DebugServer.php (1)

615-673: ⚠️ Potential issue | 🟠 Major

Keep diff baselines aligned to recorded steps when watch filtering skips execution.
previousVariables is updated before you decide to skip a step, so the next recorded diff is relative to an unrecorded step; downstream can’t reconstruct state across recorded steps. Consider updating the baseline only when a step is actually recorded (or switch to full recording when watchChanged).

🛠️ Suggested fix (update baseline only on recorded steps)

-                $previousVariables = $currentVariables;
+                // defer baseline update until we actually record

-                $previousVariables = $currentVariables;
+                // defer baseline update until we actually record

             $steps[] = $step;
+            $previousVariables = $currentVariables;
             $recordedCount++;

[koriym]

@coderabbitai Both issues addressed in b8e5b8f:

1. Content hash for array/object mutations: evaluateWatchExpression() now uses property_get to fetch child elements and generates an md5-based content hash. Output format: array(2)#064e1ab3. Internal mutations (element changes, additions, removals) are now detected.

2. maxSteps separation: Added $recordedCount counter. maxSteps now limits recorded steps only. A separate safety cap (maxSteps * 10) prevents infinite loops when watches are active.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the implementation of content-based hashing in evaluateWatchExpression()
echo "=== Checking evaluateWatchExpression() implementation ==="
ast-grep --pattern $'private function evaluateWatchExpression($$$) {
  $$$
}'

echo -e "\n=== Checking for property_get usage and content hashing ==="
rg -n -A 10 "property_get" src/DebugServer.php

echo -e "\n=== Checking for recordedCount counter and safety cap ==="
rg -n -C 3 "recordedCount" src/DebugServer.php

echo -e "\n=== Checking for maxSteps \* 10 safety cap logic ==="
rg -n -C 3 "maxSteps.*10|10.*maxSteps" src/DebugServer.php

Length of output: 3297

---

🏁 Script executed:

#!/bin/bash
# Check for the content hash generation with md5
echo "=== Checking for md5-based content hashing ==="
rg -n -B 5 -A 15 "getWatchContentHash" src/DebugServer.php

echo -e "\n=== Checking the format of hash output (array(2)#064e1ab3) ==="
rg -n -C 5 "array\(\d+\)#|#[a-f0-9]{8}" src/DebugServer.php

Length of output: 1994

---

🏁 Script executed:

#!/bin/bash
# Check for md5 hash generation in getWatchContentHash
echo "=== Checking complete getWatchContentHash implementation ==="
rg -n -A 30 "private function getWatchContentHash" src/DebugServer.php | head -50

echo -e "\n=== Checking for md5 usage ==="
rg -n "md5" src/DebugServer.php

Length of output: 1472

---

@koriym Perfect! I've verified both fixes in commit b8e5b8f:

✅ Content-based hashing (lines 1124-1160):

• getWatchContentHash() uses property_get with depth 0 to fetch all child elements
• Builds a content signature by iterating through child properties (name + value)
• Returns first 8 chars of md5 hash: substr(md5($signature), 0, 8)
• Output format confirmed: array(2)#064e1ab3 for arrays, object: ClassName#064e1ab3 for objects

✅ maxSteps separation (lines 501, 514, 688, 691):

• $recordedCount tracks only recorded steps (not executed steps)
• maxExecutedSteps = $maxSteps * 10 safety cap prevents infinite loops when watches are active
• Clear separation: $recordedCount for user-visible limit, $stepCount for safety cap

Both implementations look solid. The content hashing will now detect internal array/object mutations, and the watch filtering won't prematurely hit the step limit! 🎯

   /\_/\
  ( o.o )  Great fixes!
   > ^ <   Watch debugging
  /|   |\  just got way
 (_|   |_) more efficient! 

---

🧠 Learnings used

Learnt from: CR
Repo: koriym/xdebug-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T16:32:32.982Z
Learning: Applies to **/*.php : Use ./bin/xstep for interactive step debugging with breakpoints and variable inspection

---

Learnt from: koriym
Repo: koriym/xdebug-mcp PR: 11
File: CLAUDE.md:422-448
Timestamp: 2025-08-27T09:06:39.195Z
Learning: Current xdebug-mcp implementation lacks cross-session step debugging capabilities (xdebug_step_into across different MCP sessions), requiring either documentation updates to reflect current limitations or additional implementation work to manage session state.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!