-
Notifications
You must be signed in to change notification settings - Fork 56
fix(web): dedupe phantom edges, for-each dive-in nav, PR #113 review nits (#145) #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| {"root":["./src/app.tsx","./src/main.tsx","./src/components/detail/activitystream.tsx","./src/components/detail/agentdetail.tsx","./src/components/detail/detailpanel.tsx","./src/components/detail/gatedetail.tsx","./src/components/detail/groupdetail.tsx","./src/components/detail/metadatagrid.tsx","./src/components/detail/outputviewer.tsx","./src/components/detail/scriptdetail.tsx","./src/components/detail/subworkflowdetail.tsx","./src/components/graph/agentnode.tsx","./src/components/graph/animatededge.tsx","./src/components/graph/egressnode.tsx","./src/components/graph/endnode.tsx","./src/components/graph/gatenode.tsx","./src/components/graph/groupnode.tsx","./src/components/graph/ingressnode.tsx","./src/components/graph/nodetooltip.tsx","./src/components/graph/scriptnode.tsx","./src/components/graph/startnode.tsx","./src/components/graph/workflowgraph.tsx","./src/components/graph/workflownode.tsx","./src/components/graph/graph-layout.ts","./src/components/layout/breadcrumbbar.tsx","./src/components/layout/errorbanner.tsx","./src/components/layout/header.tsx","./src/components/layout/outputpane.tsx","./src/components/layout/replaybar.tsx","./src/components/layout/resizablelayout.tsx","./src/components/layout/statusbar.tsx","./src/components/layout/yamlviewer.tsx","./src/hooks/use-deep-link.ts","./src/hooks/use-elapsed-timer.ts","./src/hooks/use-replay.ts","./src/hooks/use-viewed-context.ts","./src/hooks/use-websocket.ts","./src/lib/constants.ts","./src/lib/utils.ts","./src/stores/workflow-store.ts","./src/types/events.ts"],"errors":true,"version":"5.9.3"} | ||
| {"root":["./src/app.tsx","./src/main.tsx","./src/components/detail/activitystream.tsx","./src/components/detail/agentdetail.tsx","./src/components/detail/detailpanel.tsx","./src/components/detail/dialogdetail.tsx","./src/components/detail/dialogengagementprompt.tsx","./src/components/detail/dialogoverlay.tsx","./src/components/detail/gatedetail.tsx","./src/components/detail/groupdetail.tsx","./src/components/detail/metadatagrid.tsx","./src/components/detail/outputviewer.tsx","./src/components/detail/scriptdetail.tsx","./src/components/detail/subworkflowdetail.tsx","./src/components/graph/agentnode.tsx","./src/components/graph/animatededge.tsx","./src/components/graph/egressnode.tsx","./src/components/graph/endnode.tsx","./src/components/graph/gatenode.tsx","./src/components/graph/groupnode.tsx","./src/components/graph/ingressnode.tsx","./src/components/graph/nodetooltip.tsx","./src/components/graph/scriptnode.tsx","./src/components/graph/startnode.tsx","./src/components/graph/workflowgraph.tsx","./src/components/graph/workflownode.tsx","./src/components/graph/graph-layout.ts","./src/components/layout/breadcrumbbar.tsx","./src/components/layout/errorbanner.tsx","./src/components/layout/header.tsx","./src/components/layout/outputpane.tsx","./src/components/layout/replaybar.tsx","./src/components/layout/resizablelayout.tsx","./src/components/layout/statusbar.tsx","./src/components/layout/yamlviewer.tsx","./src/hooks/use-deep-link.ts","./src/hooks/use-elapsed-timer.ts","./src/hooks/use-replay.ts","./src/hooks/use-viewed-context.ts","./src/hooks/use-websocket.ts","./src/lib/constants.ts","./src/lib/utils.ts","./src/stores/workflow-store.ts","./src/types/events.ts"],"version":"5.9.3"} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -443,24 +443,25 @@ def _is_proactor_shutdown_race(self, context: dict[str, Any]) -> bool: | |
| Returns True only when all of: | ||
| - The exception is ``AssertionError`` | ||
| - The uvicorn server is in shutdown state (``should_exit`` is set) | ||
| - The traceback (if available) originates from asyncio internals | ||
| - The traceback is present and the deepest frame originates from | ||
| asyncio internals | ||
| """ | ||
| exc = context.get("exception") | ||
| if not isinstance(exc, AssertionError): | ||
| return False | ||
| if self._server is None or not getattr(self._server, "should_exit", False): | ||
| return False | ||
| # Extra safety: check traceback originates from asyncio, not user code | ||
| # Require an asyncio traceback frame so unrelated AssertionErrors | ||
| # raised during shutdown (e.g., from a workflow callback finishing | ||
| # late) propagate to the default handler instead of being silently | ||
| # swallowed. Issue #145 (I3). | ||
| import traceback as tb_mod | ||
|
|
||
| tb = exc.__traceback__ | ||
| if tb is not None: | ||
| frames = tb_mod.extract_tb(tb) | ||
| if frames and "asyncio" in frames[-1].filename: | ||
| return True | ||
| # If no traceback but server is shutting down, still suppress — | ||
| # the only known source of AssertionError during shutdown is this race. | ||
| return True | ||
| if tb is None: | ||
| return False | ||
| frames = tb_mod.extract_tb(tb) | ||
| return bool(frames) and "asyncio" in frames[-1].filename | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Issue #145 calls out both silent-swallow sites — except AssertionError:
if self._server is not None and getattr(self._server, "should_exit", False):
logger.debug(...) # Suppressed silently
else:
raiseSuggested options:
|
||
|
|
||
| def _loop_exception_handler( | ||
| self, loop: asyncio.AbstractEventLoop, context: dict[str, Any] | ||
|
|
@@ -483,12 +484,14 @@ async def _guarded_serve(self) -> None: | |
|
|
||
| If ``serve()`` itself raises ``AssertionError`` during shutdown | ||
| (rather than the exception surfacing through a callback), this | ||
| wrapper suppresses it. | ||
| wrapper applies the same asyncio-frame gate used in | ||
| ``_loop_exception_handler`` to avoid swallowing unrelated errors. | ||
| """ | ||
| try: | ||
| await self._server.serve() | ||
| except AssertionError: | ||
| if self._server is not None and getattr(self._server, "should_exit", False): | ||
| except AssertionError as exc: | ||
| ctx: dict[str, Any] = {"exception": exc} | ||
| if self._is_proactor_shutdown_race(ctx): | ||
| logger.debug( | ||
| "Suppressed proactor accept-loop AssertionError during server shutdown" | ||
| ) | ||
|
|
||
This file was deleted.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit/Suggestion — Edge dedupe is lossy on
whenmetadata.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 nowhen. But for two-conditional routes to the same target (e.g.when: successthenwhen: 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.:Minor UX nit, not blocking.