Skip to content

fix: flatten JS/TS template-literal URLs to {} placeholders - #1008

Merged
DeusData merged 1 commit into
DeusData:mainfrom
CharlesQueiroz:fix/template-string-urls
Jul 31, 2026
Merged

DeusData merged 1 commit into
DeusData:mainfrom
CharlesQueiroz:fix/template-string-urls

Conversation

@CharlesQueiroz

Copy link
Copy Markdown
Contributor

Fixes #1006.

Root cause

Client-side URL extraction keyed on static string node kinds only (is_string_like / is_string_node); template_string was never included, so any URL built with a template literal was invisible:

fetch(`/api/v1/things/${id}`)          // no HTTP_CALLS, no Route
return `/api/v1/things/${id}/detail`   // no string_ref

Meanwhile the server side normalizes path params to {} (__route__GET__/api/v1/things/{}), so the join key existed on one side only and cross-repo route matching missed every parameterized endpoint.

Fix

New cbm_template_string_text() (helpers.c) flattens a template_string node: string_fragment children verbatim, each template_substitution becomes the canonical {} placeholder. Wired into the four extraction points that previously accepted only static strings:

  • extract_positional_url and extract_string_value (extract_calls.c): call-argument URLs, keyword and positional.
  • handle_string_refs (extract_unified.c): URL-shaped string refs from const/return positions.
  • handle_string_constants (extract_unified.c): module-level const lookup table.

Template literals now behave exactly like the equivalent static literal with {} in place of each interpolation; behavior for static strings is unchanged.

Validation

  • New regression test extract_ts_template_string_url_issue1006: fetch(/api/v1/things/${id}) yields first_string_arg == "/api/v1/things/{}", and the return-position template lands in string_refs as "/api/v1/things/{}/detail".
  • Full suite: 5986 passed, 1 skipped, 0 failures.
  • End-to-end fixture (TanStack-style hook, the failing shape from JS/TS: template-literal URLs in fetch/query hooks produce no HTTP_CALLS or Route nodes (static literals work) #1006) indexed with the patched binary now yields __route__ANY__/api/v1/things/{} and the queryFn -HTTP_CALLS-> /api/v1/things/{} edge.

@CharlesQueiroz
CharlesQueiroz requested a review from DeusData as a code owner July 10, 2026 16:26
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 10, 2026
@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges ux/behavior Display bugs, docs, adoption UX priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 10, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for putting this together. I have triaged it into 0.9.1-rc as the fix candidate for #1006. Review focus will be template-literal flattening semantics, placeholder handling, and confirming that existing static-literal fetch/query extraction stays unchanged.

@DeusData

Copy link
Copy Markdown
Owner

Reviewed and verified locally — the approach is right, the flattening semantics match the server-side placeholder exactly, and static-literal behavior is unchanged. Three things before merge, two mechanical and one small hardening:

Verified working (your branch merged onto current main):

1. DCO (blocking): commit d2868cd lacks a Signed-off-by matching its author. git commit --amend -s (keeping author email = sign-off email) and force-push fixes it.

2. clang-format (blocking): two lines need wrapping — make lint-format locally, or apply:

// extract_unified.c:527
    char *value =
        flat_value ? (char *)flat_value : cbm_node_text(ctx->arena, value_node, ctx->source);
// extract_unified.c:578 (initializer wraps)
            .enclosing_func_qn =
                state->enclosing_func_qn ? state->enclosing_func_qn : ctx->module_qn,

3. Hardening (please include): in extract_positional_url, the flattened template text bypasses strip_and_validate_string_arg, which for quoted strings rejects control characters and over-long args. Template literals legally contain raw newlines (multiline SQL/HTML passed to a fetch-like callee would mint a junk route containing a newline). Routing the flat text through the same validator closes that — it's quote-agnostic, so it's a one-liner:

    if (strcmp(ak, "template_string") == 0) {
        const char *flat = cbm_template_string_text(ctx->arena, arg, ctx->source);
        if (flat) {
            return strip_and_validate_string_arg(ctx->arena, (char *)flat);
        }
    }

(Also noticed escape_sequence fragments inside templates are silently dropped by the flattener — fine for URLs, not worth complicating; just mentioning it's a known, accepted simplification.)

With those three in and CI green this is good to merge. Nice work on keying the placeholder to the server-side canonical form rather than inventing a new one.

@CharlesQueiroz

Copy link
Copy Markdown
Contributor Author

Applied all three points: sign-off added to the commit, the two lines in extract_unified.c are now clang-format clean, and the flattened template text goes through strip_and_validate_string_arg in extract_positional_url as suggested. Full suite passes locally with the regression test for #1006 intact.

@DeusData

Copy link
Copy Markdown
Owner

Reviewed and verified — this is ready on the merits, just needs a quick rebase.

Verified on your latest commit (007ee18) merged onto current main: extraction suite green (extract_ts_template_string_url_issue1006 passes), the validator hardening is in place (strip_and_validate_string_arg on the flattened template text in extract_positional_url, so a multiline template can't mint a junk route), and end-to-end a real index turns fetch(\/api/v1/things/${id}/detail`)intoroute__ANY/api/v1/things/{}/detail` while the static-literal route stays byte-identical. lint-ci clean. All three of my earlier asks are addressed — thank you.

The only thing blocking merge: your sibling PR #1007 (JAX-RS, same author) just merged, and since both touch extract_calls.c/extract_unified.c, GitHub now shows this as conflicting. Could you rebase onto current main? The conflict should be trivial (adjacent additions). Once it's mergeable and green I'll merge immediately — no further review needed.

…#1006)

Client-side URL extraction only recognized static string literals; any
template literal was silently skipped, so parameterized endpoints never
produced HTTP_CALLS edges or Route nodes and cross-repo route matching
missed them (the server side already normalizes path params to {}).

New cbm_template_string_text() flattens a template_string node: string
fragments verbatim, each ${...} substitution becomes {}. Wired into:
- extract_positional_url / extract_string_value (call-arg URLs)
- handle_string_refs (URL-shaped refs from const/return positions)
- handle_string_constants (module-level const lookups)

`/api/v1/things/${id}` now yields __route__ANY__/api/v1/things/{} and
the enclosing function gets the HTTP_CALLS edge, joining the canonical
placeholder shape of server-side routes.

Signed-off-by: Charles Queiroz <fcqueiroz@liquibase.com>
@CharlesQueiroz
CharlesQueiroz force-pushed the fix/template-string-urls branch from 007ee18 to a029b3a Compare July 16, 2026 17:51
@CharlesQueiroz

Copy link
Copy Markdown
Contributor Author

Rebased onto current main. The conflict was the adjacent RUN_TEST registration from #1007 in tests/test_extraction.c, resolved keeping both. Full suite on the rebased branch: 6358 passed, 1 skipped, 0 failures. lint-format clean.

@DeusData

Copy link
Copy Markdown
Owner

A note so this PR's status is not a mystery: #1010 physically contains this PR's head commit a029b3a — it is stacked on top of it. So merging #1010 would land this work too and auto-close this PR.

That means the review conversation has effectively moved to #1010, and I did not want you to think this one was being ignored.

Your cbm_template_string_text() flattening was reviewed as part of that stack and it is sound: bounds-checked before every write, arena-allocated with the copy-out done correctly, no lifetime or multithreading hazard, and wired consistently through call-arg extraction, module-const collection and string_refs so client template-literal URLs converge on the same canonical {} shape the server side already uses. That last point is the valuable bit — it is what makes the two halves of a route actually meet in the graph.

#1010 needs one change before either can land, and it is not in your diff: its URL-builder recording has no language gate, so on C and Go a helper returning "/etc/app/conf" is treated as a URL builder and mints a fabricated Route node plus an HTTP_CALLS edge. Details are in my comment there. Once that gate is added, both land together.

One small asymmetry worth knowing, since it lives in your half: builder_template_text truncates query strings at ?, but the general cbm_template_string_text does not. So fetch(`/api/x?y=${z}`) keeps ?y={} while the identical URL reached through a builder loses it — which can produce two Route QNs for one route depending purely on call syntax. Not a blocker and arguably belongs downstream, but it is the kind of thing that is much cheaper to align now than after both shapes are in people's graphs.

Nothing needed from you right now unless you and @CharlesQueiroz would rather unstack them — your call, and either way is fine by us.

@DeusData
DeusData merged commit facd32b into DeusData:main Jul 31, 2026
35 of 37 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged — thank you, and good news on the CI too.

Your checks were never actually broken. I re-ran them and everything came back green with no change to your code. The earlier red was a CodeQL gate that polled out 57 seconds before CodeQL itself succeeded on the same commit — pure infrastructure timing. You were never being asked to fix anything; the PR just looked red. Apologies for the time it spent looking like your problem.

On the change itself. cbm_template_string_text() flattens a JS/TS template literal to text with each ${…} becoming {}, and it is wired consistently through the three places that matter — call-argument extraction, module-constant collection, and string_refs. That last part is what makes it valuable rather than merely correct: client-side template-literal URLs now converge on the same canonical {} shape the server side already produces, which is what lets the two halves of a route actually meet in the graph instead of sitting there as two unrelated strings.

The implementation was reviewed carefully as part of the #1010 stack, since that PR builds on this commit. It is bounds-checked before every write, arena-allocated with the copy-out done correctly, and carries no lifetime or cross-thread hazard — the map is per-file and walk-local. extract_ts_template_string_url_issue1006 is binding.

One asymmetry worth knowing about, not a blocker and arguably belonging downstream: builder_template_text in #1010 truncates query strings at ?, while cbm_template_string_text does not. So fetch(`/api/x?y=${z}`) keeps ?y={} in first_string_arg, whereas the identical URL reached through a builder loses it — which can yield two Route QNs for one route depending purely on call syntax. Worth aligning at some point; much cheaper now than once both shapes are in people's graphs.

On #1010: it needs one change before it can follow — its URL-builder recording has no language gate, so on C and Go a helper returning "/etc/app/conf" gets treated as a URL builder and mints a fabricated Route node plus an HTTP_CALLS edge. Details are in my comment there. With this merged, that PR now reduces to just the builder work, which should make the gate change simpler to land.

Thanks for the careful work on both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JS/TS: template-literal URLs in fetch/query hooks produce no HTTP_CALLS or Route nodes (static literals work)

2 participants