Populate hitBreakpointIds in the DAP stopped event

[Alexandre Perez (aperez)]

The DAP stopped event declares hitBreakpointIds so clients can show which breakpoint caused a stop, but debugpy never filled it in, so the field was permanently absent and a stop could not be attributed to a specific breakpoint.

Both tracing backends now record the id of the breakpoint being waited on, covering line, function, and plugin (template) breakpoints. The id is recorded immediately before the thread waits, so it is set only when the thread really suspends, and cleared on resume so a later stop cannot inherit it. It is attached only to breakpoint and function breakpoint stops, because the pause timeout can re-report a thread that is still parked at a breakpoint.

Function breakpoints carried no id at all, so FunctionBreakpoint now takes breakpoint_id as its first parameter, matching LineBreakpoint.

The hitBreakpointIds list only ever holds one id. pydevd keeps at most one breakpoint per resolved line (consolidate_breakpoints) and discards the rest. Making the list meaningful needs one-to-many breakpoint storage, which is separate work.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Rich Chiodo (rchiodo)]

🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR.

[Rich Chiodo (rchiodo)]

I'm guessing you need this for something other than VS code? Otherwise it's not necessary?

[Rich Chiodo (rchiodo)]

Approved via Review Center.

[Alexandre Perez (aperez)]

I'm guessing you need this for something other than VS code? Otherwise it's not necessary?

Rich Chiodo (@rchiodo) My primary motivation is because we have AI agents that interface with DAP directly and it would be easier to retrieve the breakpoint id from a stopped event than having to figure out what was hit by looking at the topmost frame in the stack.
My understanding is that VS Code also benefits from this (although I assume it also has the aforementioned logic to fallback to the topmost frame). VS Code uses hitBreakpointIds for activating dependent / triggered breakpoints and for focus / selection. These are just two locations i was able to pinpoint but it may also be used elsewhere.

[Stella Huang (StellaHuang95)]

Approved via Review Center.

[Rich Chiodo (rchiodo)]

My understanding is that VS Code also benefits from this (although I assume it also has the aforementioned logic to fallback to the topmost frame). VS Code uses hitBreakpointIds for activating dependent / triggered breakpoints and for focus / selection. These are just two locations i was able to pinpoint but it may also be used elsewhere.

I guess I'm scared of breaking VS code. This means if the breakpoint ids are incorrect or not interpreted correctly VS code behavior could be affected. None of the tests in debugpy would exercise this path. I'm guessing the AI you're using wouldn't indicate that in anyway through DAP so these could be sent only in that case?

[Rich Chiodo (rchiodo)]

I'll have Copilot do an analysis of the code in VS code to see what affect this might have if any.

[Alexandre Perez (aperez)]

I guess I'm scared of breaking VS code. This means if the breakpoint ids are incorrect or not interpreted correctly VS code behavior could be affected. None of the tests in debugpy would exercise this path. I'm guessing the AI you're using wouldn't indicate that in anyway through DAP so these could be sent only in that case?

Rich Chiodo (@rchiodo) Would you prefer I create a launch config flag that controls this behavior and defaults to false?

[Rich Chiodo (rchiodo)]

I guess I'm scared of breaking VS code. This means if the breakpoint ids are incorrect or not interpreted correctly VS code behavior could be affected. None of the tests in debugpy would exercise this path. I'm guessing the AI you're using wouldn't indicate that in anyway through DAP so these could be sent only in that case?

Rich Chiodo (Rich Chiodo (@rchiodo)) Would you prefer I create a launch config flag that controls this behavior and defaults to false?

Wouldn't that break your scenario? You'd have to tell your models to include the setting. Copilot analysis of VS code makes it sound relatively harmless. The only problem is if we return a partial list when VS code would have generated a full list (like when there's more than one breakpoint on the same line.)

[Alexandre Perez (aperez)]

Wouldn't that break your scenario? You'd have to tell your models to include the setting.

Yeah I'd have to make sure our launch configs passed the setting, which I think is doable if we want to avoid changing any behavior in the case where VS Code is the client.

[Rich Chiodo (rchiodo)]

I dug into how VS Code actually consumes hitBreakpointIds to gauge the blast radius of populating it, and there's one edge case worth guarding before this merges.

How VS Code uses it: it's read in exactly one place — DebugSession.handleStop() -> enableDependentBreakpoints() in src/vs/workbench/contrib/debug/browser/debugSession.ts. It drives only the "triggered breakpoints" feature (a breakpoint that arms once another is hit). It has no effect on stack-frame focus, editor reveal, the gutter, the call stack, or the breakpoints view, and there are no asserts — bad values fail silently. So for anyone not using triggered breakpoints, this change is invisible (no crash, no mis-highlight).

The catch: when the field is present, VS Code takes an eager id-based path and skips its position-based fallback:

if (event.hitBreakpointIds) {                 // truthy -> id-based path
    this._waitToResume = this.enableDependentBreakpoints(event.hitBreakpointIds);
}
...
if (!event.hitBreakpointIds) {                // fallback ONLY runs when falsy
    this._waitToResume = this.enableDependentBreakpoints(thread);
}

The id path matches bp.getIdFromAdapter(session.id) against the reported ids; the fallback matches all breakpoints at the stop position. So a present-but-wrong/partial id is strictly worse than an absent one for triggered breakpoints. (One thing you already got right: you send None, never [], in the no-id case — an empty array is truthy in JS and would wrongly suppress the fallback.)

Where that bites here: consolidate_breakpoints keeps one breakpoint per resolved line (break_dict[pybreakpoint.line] = pybreakpoint) and drops the rest, but every id stays in file_to_id_to_line_breakpoint. When two ids collapse onto the same resolved line (e.g. two source lines that both adjust to the same executable line, or column breakpoints), you report only the survivor's id. If a triggered breakpoint depended on the dropped id, VS Code won't arm it — and since the field is present, the position fallback that previously would have matched it is skipped.

Suggested fix (no cython regen needed): omit hitBreakpointIds when the hit line is ambiguous, so VS Code falls back to position matching (exactly the pre-PR behaviour). The dropped ids are still recoverable from file_to_id_to_line_breakpoint, so ambiguity is detectable at emit time. Concrete code in the inline comments below. Narrow feature impact, but cheap to make robust.

[Rich Chiodo (rchiodo)]

Consider routing both emission sites through a small helper so a consolidated/ambiguous line omits the ids (letting VS Code fall back to matching by stop position) rather than reporting a possibly-wrong survivor id:

hitBreakpointIds=self._get_hit_breakpoint_ids(py_db, info, stop_reason),

with this helper on the factory class:

def _get_hit_breakpoint_ids(self, py_db, info, stop_reason):
    if stop_reason not in ("breakpoint", "function breakpoint"):
        return None
    hit_breakpoint_ids = info.hit_breakpoint_ids
    if not hit_breakpoint_ids:
        return None
    # Line breakpoints are consolidated per resolved line, so if several ids
    # collapse onto the hit line only one survives; reporting it alone is
    # unreliable and suppresses the client's position-based fallback. Function
    # and plugin/template breakpoints aren't line-consolidated, so keep theirs.
    if stop_reason == "breakpoint" and any(
        py_db.has_line_breakpoint_id_collision(bp_id) for bp_id in hit_breakpoint_ids
    ):
        return None
    return hit_breakpoint_ids

and a PyDB helper next to consolidate_breakpoints in pydevd.py:

def has_line_breakpoint_id_collision(self, breakpoint_id):
    # True if more than one line breakpoint resolves to the same (file, line) as
    # the given id -- meaning consolidate_breakpoints() dropped some and a single
    # reported id can't be trusted.
    for id_to_breakpoint in self.file_to_id_to_line_breakpoint.values():
        pybreakpoint = id_to_breakpoint.get(breakpoint_id)
        if pybreakpoint is None:
            continue
        line = pybreakpoint.line
        found = 0
        for other in id_to_breakpoint.values():
            if other.line == line:
                found += 1
                if found > 1:
                    return True
        return False
    return False

This keeps the precise id in the common case and only degrades to the old position-based behaviour when the line is genuinely ambiguous. Plugin/template (django/jinja) ids aren't in file_to_id_to_line_breakpoint, so they're left untouched and the test_django assertion still holds.

[Rich Chiodo (rchiodo)]

Same consolidation caveat as the single-notification path — worth routing this per-thread emission through the same self._get_hit_breakpoint_ids(py_db, info, stop_reason) helper so both sites behave identically when ids collapse onto one line.

[Rich Chiodo (rchiodo)]

Copilot had some ideas to handle the off case where two breakpoints map to the same line. I'll have it comment some suggestions. But otherwise seems okay to go ahead with this.

[Rich Chiodo (rchiodo)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Rich Chiodo (rchiodo)]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Stella Huang (StellaHuang95)]

Approved via Review Center.