diff --git a/src/playground/src/api.ts b/src/playground/src/api.ts index d37ed61..9d9c85b 100644 --- a/src/playground/src/api.ts +++ b/src/playground/src/api.ts @@ -155,20 +155,24 @@ class ApiError extends Error { } async function get(path: string): Promise { - const res = await fetch(`/api${path}`); - if (!res.ok) { - const body = await res.json().catch(() => ({})); - throw new ApiError(res.status, body.detail ?? body.error ?? res.statusText); - } - return res.json() as Promise; + return parse(await fetch(`/api${path}`)); } async function post(path: string): Promise { - const res = await fetch(`/api${path}`, { method: "POST" }); + return parse(await fetch(`/api${path}`, { method: "POST" })); +} + +// Turn a Response into JSON, with clear errors. A non-JSON body on a 200 (e.g. an +// unmatched /api route falling through to the SPA's index.html) becomes a plain +// ApiError instead of a cryptic "did not match the expected pattern" JSON crash. +async function parse(res: Response): Promise { if (!res.ok) { const body = await res.json().catch(() => ({})); throw new ApiError(res.status, body.detail ?? body.error ?? res.statusText); } + if (!res.headers.get("content-type")?.includes("application/json")) { + throw new ApiError(res.status, "unexpected non-JSON response from the API"); + } return res.json() as Promise; } @@ -190,13 +194,15 @@ export const api = { get(`/graph/overview?expanded=${encodeURIComponent(expanded)}`), ego: (qualified_name: string) => get(`/graph/ego?qualified_name=${q(qualified_name)}`), - node: (qualified_name: string) => get(`/node/${q(qualified_name)}`), + // qualified_name goes in the query string (a file node's qn is a path with + // slashes; a path segment would break routing — see serve/routes.py). + node: (qualified_name: string) => get(`/node?qualified_name=${q(qualified_name)}`), rationaleRead: (qualified_name: string) => - get(`/node/${q(qualified_name)}/rationale`), + get(`/node/rationale?qualified_name=${q(qualified_name)}`), rationaleGenerate: (qualified_name: string) => - post(`/node/${q(qualified_name)}/rationale`), + post(`/node/rationale?qualified_name=${q(qualified_name)}`), evidence: (qualified_name: string, limit = 20) => - get(`/node/${q(qualified_name)}/evidence?limit=${limit}`), + get(`/node/evidence?qualified_name=${q(qualified_name)}&limit=${limit}`), history: (path: string, limit = 20) => get(`/history?path=${encodeURIComponent(path)}&limit=${limit}`), }; diff --git a/src/whygraph/serve/routes.py b/src/whygraph/serve/routes.py index 877aebf..d355c6b 100644 --- a/src/whygraph/serve/routes.py +++ b/src/whygraph/serve/routes.py @@ -139,10 +139,16 @@ def graph_ego( # ---- node detail --------------------------------------------------------- +# +# `qualified_name` travels as a QUERY parameter, not a path segment: a CodeGraph +# `file` node's qualified_name is a path (e.g. "src/pkg/a.py"), so a path segment +# would carry slashes — uvicorn decodes `%2F` to `/`, the single-segment route +# stops matching, and the request falls through to the SPA catch-all (returning +# index.html). A query param sidesteps that entirely. -@router.get("/node/{qualified_name}") -def node_detail(qualified_name: str) -> dict: +@router.get("/node") +def node_detail(qualified_name: str = Query(...)) -> dict: """Identity + typed relationships for a symbol (the Relationships tab).""" with _open_graph() as graph: symbol = graph.symbol(qualified_name) @@ -157,8 +163,8 @@ def node_detail(qualified_name: str) -> dict: } -@router.get("/node/{qualified_name}/rationale") -def rationale_read(qualified_name: str) -> dict: +@router.get("/node/rationale") +def rationale_read(qualified_name: str = Query(...)) -> dict: """Cache-only rationale read — never calls an LLM (the resolved Q3 split). Returns ``{status: "cached", ...card}`` on a cache hit, @@ -183,8 +189,8 @@ def rationale_read(qualified_name: str) -> dict: } -@router.post("/node/{qualified_name}/rationale") -def rationale_generate(qualified_name: str) -> dict: +@router.post("/node/rationale") +def rationale_generate(qualified_name: str = Query(...)) -> dict: """Generate + cache a rationale card (the explicit "Generate" action). Runs :func:`whygraph_rationale_brief` verbatim — the same generate-and-cache @@ -195,8 +201,10 @@ def rationale_generate(qualified_name: str) -> dict: return {"status": "cached", **card} -@router.get("/node/{qualified_name}/evidence") -def evidence(qualified_name: str, limit: int = Query(20, ge=1, le=100)) -> dict: +@router.get("/node/evidence") +def evidence( + qualified_name: str = Query(...), limit: int = Query(20, ge=1, le=100) +) -> dict: """Historical evidence for a symbol (the Evidence tab).""" return whygraph_evidence_for(qualified_name=qualified_name, limit=limit) diff --git a/tests/test_serve_api.py b/tests/test_serve_api.py index 8d7f36a..4740221 100644 --- a/tests/test_serve_api.py +++ b/tests/test_serve_api.py @@ -196,7 +196,7 @@ def test_overview_expanded_reveals_files(serve_client) -> None: def test_node_detail_groups_relations(serve_client) -> None: - body = serve_client.get("/api/node/pkg.a.A.m").json() + body = serve_client.get("/api/node?qualified_name=pkg.a.A.m").json() assert body["symbol"]["qualified_name"] == "pkg.a.A.m" rel = body["relations"] assert [c["qualified_name"] for c in rel["callers"]] == ["pkg.b.caller"] @@ -205,7 +205,21 @@ def test_node_detail_groups_relations(serve_client) -> None: def test_node_detail_404_for_unknown(serve_client) -> None: - assert serve_client.get("/api/node/pkg.nope").status_code == 404 + assert serve_client.get("/api/node?qualified_name=pkg.nope").status_code == 404 + + +def test_node_detail_handles_file_node_with_slashes_in_qn(serve_client) -> None: + # A `file` node's qualified_name is a path with slashes (e.g. "src/pkg/a.py"). + # As a query param this must resolve cleanly and return JSON — not fall through + # to the SPA (which previously returned index.html with a 200). + r = serve_client.get("/api/node?qualified_name=src/pkg/a.py") + assert r.status_code == 200 + assert r.headers["content-type"].startswith("application/json") + body = r.json() + assert body["symbol"]["qualified_name"] == "src/pkg/a.py" + assert body["symbol"]["kind"] == "file" + # A file contains its class(es); no callers/callees. + assert [c["qualified_name"] for c in body["relations"]["children"]] == ["pkg.a.A"] # ---- rationale split (the resolved Q3 design) ---------------------------- @@ -220,7 +234,7 @@ def test_rationale_get_no_evidence_makes_no_llm_call(serve_client, monkeypatch) gen = mock.Mock() monkeypatch.setattr(routes, "whygraph_rationale_brief", gen) - body = serve_client.get("/api/node/pkg.a.A.m/rationale").json() + body = serve_client.get("/api/node/rationale?qualified_name=pkg.a.A.m").json() assert body["status"] == "no_evidence" gen.assert_not_called() @@ -236,7 +250,7 @@ def test_rationale_get_not_generated_makes_no_llm_call( gen = mock.Mock() monkeypatch.setattr(routes, "whygraph_rationale_brief", gen) - body = serve_client.get("/api/node/pkg.a.A.m/rationale").json() + body = serve_client.get("/api/node/rationale?qualified_name=pkg.a.A.m").json() assert body["status"] == "not_generated" gen.assert_not_called() @@ -267,7 +281,7 @@ def test_rationale_get_returns_cached_card(serve_client, monkeypatch) -> None: gen = mock.Mock() monkeypatch.setattr(routes, "whygraph_rationale_brief", gen) - body = serve_client.get("/api/node/pkg.a.A.m/rationale").json() + body = serve_client.get("/api/node/rationale?qualified_name=pkg.a.A.m").json() assert body["status"] == "cached" assert body["purpose"] == "the purpose" @@ -283,7 +297,7 @@ def test_rationale_post_calls_brief_verbatim(serve_client, monkeypatch) -> None: gen = mock.Mock(return_value=card) monkeypatch.setattr(routes, "whygraph_rationale_brief", gen) - body = serve_client.post("/api/node/pkg.a.A.m/rationale").json() + body = serve_client.post("/api/node/rationale?qualified_name=pkg.a.A.m").json() assert body["status"] == "cached" assert body["purpose"] == "generated purpose"