fix(web): dedupe phantom edges, for-each dive-in nav, PR #113 review nits (#145)

[PolyphonyRequiem]

Follow-ups for #113 — bundles four fixes surfaced after merge.

1. Phantom edges in subworkflow graphs (commit fcab784)

Workflows that use a conditional + bare catch-all route to the same target (e.g. - to: when: X followed by - to: ) emitted two RouteEdges with identical (source, target). graph-layout.ts rendered each as a separate edge, which dagre then laid out as overlapping/diverging strands — hence the visual "phantom" edges. Fix: dedupe by (from, to), first route wins (matches engine first-match-wins).

Repro: open dashboard at root, click into implementation subworkflow → confirmed gone after the fix.

2. For-each iteration dive-in (this commit)

The detail panel for a for_each group listed iterations but offered no way to navigate into a specific iteration's subworkflow context. pg_execution_group[0] could not be linked into. Added a Layers icon button per iteration row that resolves the slot key (<group>[<key>]) against subworkflowContexts and calls navigateIntoSubworkflow. Click + Enter/Space supported.

3. Tighten _is_proactor_shutdown_race — #145 (I3)

Per Jason's review: the proactor race gate fell back to permissive True when no traceback was attached, which could swallow unrelated AssertionErrors raised by user/workflow code during shutdown. Now requires the deepest traceback frame to originate from asyncio internals. Tests updated to assert the tightened contract (asyncio frame → True; non-asyncio frame → False; missing traceback → False).

4. Doc nits — #145 (S2, S3)

• engine/workflow.py: comment on why _check_interrupt clears the interrupt event before raising (single-listener reset semantics, no race).
• stores/workflow-store.ts: docstring on resolveSlotPath noting that newest-wins semantics make older for-each iterations unreachable by stale slot paths.

Validation

• 137 web + engine tests pass
• ruff check + ruff format --check clean
• Frontend bundle rebuilt; index.html references new asset filenames

Refs

• Closes Follow-up from PR #113: tighten proactor AssertionError suppression + small comment/docstring nits #145
• Builds on feat: breadcrumb navigation, subworkflow isolation, and stop reliability #113

🤖 Co-authored with Copilot CLI

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@bf0e8e9). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #146   +/-   ##
=======================================
  Coverage        ?   85.33%           
=======================================
  Files           ?       56           
  Lines           ?     8109           
  Branches        ?        0           
=======================================
  Hits            ?     6920           
  Misses          ?     1189           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jrob5756]

two small fixes

[jrob5756]

_guarded_serve retains the same permissive AssertionError handling that was just tightened here.

Issue #145 calls out both silent-swallow sites — _loop_exception_handler and _guarded_serve (lines 443-458 below). The wrapper still catches any AssertionError whenever should_exit is set, with no traceback inspection:

except AssertionError:
    if self._server is not None and getattr(self._server, "should_exit", False):
        logger.debug(...)  # Suppressed silently
    else:
        raise

Suggested options:

• Apply the same asyncio-frame check inline in the except, or
• Refactor: synthesize a context dict from the exception and call self._is_proactor_shutdown_race(ctx) so both sites share one gate.

[jrob5756]

Nit/Suggestion — Edge dedupe is lossy on when metadata.

The surviving edge keeps data: { when: r.when } from the first route only. For the documented repro (conditional + bare catch-all) that's fine — the second route has no when. But for two-conditional routes to the same target (e.g. when: success then when: error), the visual edge will display only [success] even though the actual graph means "go to $end on either condition."

Consider clearing when (or marking it '*' / 'multiple') when collapsing routes with non-equivalent conditions, e.g.:

const existing = seenPairs.get(pairKey);
if (existing && existing !== r.when) {
  // Multiple distinct conditions collapse — drop the label.
  flowEdges[existing.idx].data = { when: undefined };
  continue;
}

Minor UX nit, not blocking.

[PolyphonyRequiem]

Addressed both review comments in commit 9c0940f:

1. _guarded_serve tightened - now calls _is_proactor_shutdown_race(ctx) with a synthesized context dict instead of the permissive should_exit-only check. Both exception-handler sites share the same strict asyncio-frame gate. Added test_guarded_serve_reraises_non_asyncio_assertion_during_shutdown to verify the tightened contract.

2. Edge dedupe preserves label accuracy - changed seenPairs from a Set to a Map tracking both the when value and the edge index. When collapsing routes with different conditions to the same target, the label is cleared (undefined) so the UI doesn't misleadingly show only one condition.

All 48 web server tests pass.

[jrob5756]

LGTM — both review findings addressed cleanly. Shared gate for the proactor race is the right call.