Conversation
Distilled from #1245 by Andrew Hundt (a slice of 1ac3c13). The route-registration suffix table knew .add_api_websocket_route but not the decorator forms FastAPI and Starlette actually use: @router.websocket("/ws") @app.websocket_route("/events") so a WebSocket endpoint produced no Route node on either resolver path and was invisible to the graph. Two entries join the table, both recorded as ANY like the existing WebSocket entry -- a socket upgrade is not an HTTP verb. Both are needed: matching is a pure suffix compare and .route does not match ...websocket_route, whose tail is _route. Upstream also changed pass_calls.c, pass_parallel.c and pipeline_internal.h. None of that is taken: main already carries the #952 sequential unresolved-callee route fallback its July base lacked, so the table entries are the only missing piece. RED before the fix, each form recorded on its own (the harness stops at the first failing assert, so the two were swapped and re-run): pipeline_python_websocket_decorators_register_routes FAIL tests/test_pipeline.c:6163: count_nodes_named(s, project, "/ws") (0) not >= 1 (1) FAIL tests/test_pipeline.c:6163: count_nodes_named(s, project, "/events") (0) not >= 1 (1) The @app.get("/health") control above those lines passed before and after. GREEN after: pipeline 282 passed; pipeline parallel extraction edge_types_probe route_canon infrascan 779 passed, 0 failed. The test drives the sequential resolver (one file). The parallel resolver reads the same table through cbm_service_pattern_route_method and its suite is green; a CBM_WORKERS=4 twin of this test is a small follow-up. Co-authored-by: Andrew Hundt <ATHundt@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
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.
Distilled from #1245 by @ahundt (a slice of 1ac3c13), carried with
Co-authored-by. #1245 stays open until its distills land.The bug
The route-registration suffix table knew
.add_api_websocket_routebut not the forms FastAPI and Starlette actually use —@router.websocket("/ws")and@app.websocket_route("/events")— so a WebSocket endpoint produced no Route node on either resolver path.The fix
Two table entries, both recorded as
ANYlike the existing WebSocket entry (a socket upgrade is not an HTTP verb). Both are needed: matching is a pure suffix compare, and.routedoes not match…websocket_route, whose tail is_route.Upstream also changed
pass_calls.c,pass_parallel.candpipeline_internal.h. None of that is taken — main already carries the #952 sequential unresolved-callee route fallback its July base lacked, so the table is the only missing piece.RED → GREEN
Each form recorded on its own (the harness stops at the first failing assert, so they were swapped and re-run):
The
@app.get("/health")control passed before and after.After:
pipeline282 passed;pipeline parallel extraction edge_types_probe route_canon infrascan779 passed, 0 failed.The test drives the sequential resolver (one file). The parallel resolver reads the same table via
cbm_service_pattern_route_methodand its suite is green; aCBM_WORKERS=4twin is a small follow-up.