feat: own the dbt Semantic Layer client, talking to the API directly

[josep-reyero]

Problem

A semantic layer call travels MCP client → lfx-mcp → lfx-lens over HTTP, and
only then to the dbt Semantic Layer. lfx-lens is the text-to-SQL service; the
semantic layer lives there for historical reasons, and the hop buys nothing,
because lfx-lens performs no per-user authorization on those routes.

What it costs is real: extra latency, an extra failure mode, and a second
release train for every semantic layer change. That last one already bit us —
lfx-mcp v0.12.0 sat inert in production until lfx-lens v0.5.5 shipped, because
the MCP tool called an endpoint the deployed lens did not yet have.

What this does

New internal/dbtsl package, peer to internal/lfxv2 and
internal/serviceapi: GraphQL transport, TTL caches, metric and dimension
metadata, the INSIGHTS_METRICS_ALLOWLIST with ranked suggestions and topic
words, the singular-stem and per-word search fallbacks, query execution, and
gated dimension value discovery. Ported from
lfx-lens/ai/services/dbt_semantic_layer.py.

De-branded tool surface. internal/tools/lens.go held both surfaces in 664
lines; it shrinks to query_lfx_lens alone, which still routes through lfx-lens
and keeps its LFXMCP_LENS_API_* settings. The semantic layer moves to
semanticlayer.go with its own config type. Tool names are unchanged — they
carry no lens branding and are already in the production allowlist and in live
client prompts.

Three new settings, LFXMCP_DBT_SL_HOST, LFXMCP_DBT_SL_ENVIRONMENT_ID
and LFXMCP_DBT_SL_TOKEN, wired in a top-level block rather than nested inside
the Auth0 client-credentials guard that wraps the lens client: this client uses
a static service token and has no Auth0 dependency, so nesting it there would
leave it unconfigured for an unrelated reason.

Design note: GraphQL for execution too

There is no Go SDK for the dbt Semantic Layer. Both reference implementations
are Python and split the transport — lfx-lens and dbt Labs' own dbt-mcp both
use the dbtsl SDK with GraphQL for metadata and Arrow Flight for execution.

This uses GraphQL for both, via createQuery then polling query for
jsonResult. That avoids taking on Arrow, gRPC and session lifecycle for no
gain at our volumes: the tool caps limit at 500, well inside the API's
1024-row page, so pagination never engages.

Measured: transport is not the bottleneck. The same query runs 22.7s cold and
1.6s warm — a 14× difference that can only be the warehouse. Metadata calls
are 0.4–1.2s. The poll loop adds ≤0.2s on fast queries and ≤2s on slow ones.

Design note: project_slug is removed

Not kept as a no-op. Its description promised the where clause was "validated
against that foundation's subtree", and the Snowflake scope check that enforced
that is gone — it queried ANALYTICS.SILVER_DIM.PROJECT_SPINE and was the only
part of the lens implementation that was not self-contained.

It was never a security boundary: the tools are staff-gated and project_slug
was optional, so a caller bypassed the check by omitting it. Keeping the
parameter while nothing enforces it would mean a caller passing
project_slug: "cncf" reads a global result as CNCF-scoped — a silent wrong
answer, the same failure class this epic exists to remove.

Scoping now goes in the where clause, which is how it always actually worked
(project_slug only ever validated your filter, it never applied one). This
freed 69 bytes of the query tool's 2048-byte description budget.

When a non-staff tier arrives, scope should be rebuilt on OpenFGA via
internal/lfxv2/access_check.go, not on a warehouse table.

Security note

The dbt client is deliberately not given the serviceapi debug transport.
That calls httputil.DumpRequestOut(req, true), which includes the
Authorization header, and production runs with debugTraffic: true — it would
print this long-lived service token into production logs.

Separately and pre-existing: the lens and onboarding clients already dump
their Auth0 bearer tokens in production under that same setting. Out of scope
here, but it should probably be its own ticket.

Verification

CI runs only MegaLinter and the license-header check, so for the record:

• make check clean; 62 tests pass, 85.4% coverage on internal/dbtsl.
• A live parity harness behind the parity build tag
  (go test -tags parity ./internal/dbtsl/) passes against the real semantic
  layer: 59/59 allowlisted metrics reachable, country__lf_region returns the
  9 expected values including 'Asia Pacific', a viet search returns
  'Viet Nam', and both search fallbacks rescue a plural and a
  natural-language query.
• End to end through the real MCP server over stdio, across every domain,
  global and CNCF-scoped: memberships, contributions, events, education,
  maintainers, project health; multi-metric cross-domain outer joins; year
  trends; MetricFlow categorical and time filters. The PII gate rejects
  user__email, and an unknown metric returns ranked suggestions.

The parity harness earned its keep — it caught three defects a stub could not:

1. dbt's published docs name the createQuery argument order; the deployed
   schema calls it orderBy. Every ordered query would have failed.
2. Metric values decoded as float64 reached the model as 4.239559e+06
   rather than 4239559. Now decoded as json.Number.
3. Callers write time grains as metric_time__year, but the GraphQL API takes
   the grain as its own field. The Arrow path hides this, so every time-series
   query would have broken.

Deploy order

1. LFXV2-2939 first (lfx-secrets-management#297) grants this service read
   access to the dbt token, and production must be synced before the
   production chart bump or the tools break there.
2. The matching values PR in lfx-v2-argocd.
3. Release v0.13.0 (minor: substantially changed tools), then bump
   apps/prod/lfx-mcp.yaml from 0.12.0.

Rollback is clean: revert the pin. lfx-lens still serves the semantic layer
routes until LFXV2-2941, which is sequenced after production verification.

Issue: LFXV2-2940

🤖 Generated with Claude Code

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[Copilot]

Pull request overview

Moves semantic-layer operations from LFX Lens into a direct dbt GraphQL client, reducing service dependencies and latency.

Changes:

• Adds the internal/dbtsl client with caching, discovery, allowlisting, and query execution.
• Separates semantic-layer tools from Lens and adds extensive tests.
• Adds server, Helm, and documentation configuration for dbt credentials.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
README.md	Documents the standalone semantic-layer tools.
internal/tools/semanticlayer.go	Implements direct semantic-layer MCP tools.
internal/tools/semanticlayer_test.go	Tests tool behavior and schemas.
internal/tools/lens.go	Removes semantic-layer routing from Lens.
internal/tools/lens_test.go	Removes migrated Lens tests.
internal/tools/csv.go	Extracts CSV argument parsing.
internal/dbtsl/similarity.go	Implements fuzzy similarity scoring.
internal/dbtsl/search.go	Adds search fallbacks.
internal/dbtsl/query.go	Implements GraphQL query execution and polling.
internal/dbtsl/parity_live_test.go	Adds live parity tests.
internal/dbtsl/metadata.go	Fetches metric and dimension metadata.
internal/dbtsl/dimensionvalues.go	Implements guarded dimension-value discovery.
internal/dbtsl/dbtsl_test.go	Adds dbt client unit tests.
internal/dbtsl/client.go	Defines the GraphQL client and transport.
internal/dbtsl/cache.go	Adds bounded TTL caches.
internal/dbtsl/allowlist.go	Defines exposed metrics and suggestions.
cmd/lfx-mcp-server/main.go	Configures and registers the dbt client.
charts/lfx-mcp/values.yaml	Adds dbt Helm values.
charts/lfx-mcp/templates/deployment.yaml	Injects dbt configuration and credentials.
AGENTS.md	Documents architecture and configuration.

[josep-reyero]

Fixed in ca8fec3, and this turned out to be the most serious finding on the PR — thank you.

You were right that nothing enforced the ceiling, and the consequence is worse than an upstream default row count. The dbt SL GraphQL API pages results at ~1024 rows. Against the live environment, total_activities grouped by activity_project_id__organization_name with no limit returned exactly 1024 rows and reported row_count: 1024 — a truncated answer indistinguishable from a complete one, which the model would then present as fact. The Arrow Flight transport the Python implementation used streams the whole result, so this is a hazard the port introduced rather than inherited.

Two fixes: the tool now defaults an omitted or negative limit to the 500 it advertises, and Query follows totalPages to the end so a paged result is never silently cut. Same query now returns 500. Test: TestSemanticLayerDefaultsTheLimitToTheAdvertisedCeiling.

[josep-reyero]

Fixed in ca8fec3. Confirmed: stdio runs on a background context and main.go sets only ReadHeaderTimeout, so nothing bounded the loop. Query now carries its own queryMaxWait budget while still honouring earlier caller cancellation.

Reviewing this also surfaced the opposite failure alongside it: pollQuery returning any error aborted immediately, so a single 502 mid-poll discarded a query still running upstream and about to succeed. The Python route retried once on transport errors, so that was a regression. The loop now absorbs a bounded number of consecutive transport errors, while a FAILED status still aborts at once — the same split the Python had between QueryFailedError and transport errors. Tests: TestQueryToleratesATransientPollFailure, TestQueryGivesUpAfterRepeatedPollFailures, TestQueryDoesNotRetryAnApplicationFailure.

[josep-reyero]

Fixed in ca8fec3, and I confirmed the premise against the live API rather than assuming it: FetchDimensions(ctx, nil) returns 295 dimensions — every dimension in the environment, so an empty metric list does exactly what you describe.

One correction to the severity: it is not reachable today. The only caller, handleSLGetDimensionValues, rejects an empty metrics before this point. But the gate is documented as an invariant of this function, and it was not one, so it now holds where it is claimed. Test: TestFetchDimensionValuesRejectsAnEmptyMetricList.

[josep-reyero]

Pushed ca8fec3, addressing the Copilot review plus an independent review pass.

The one that matters: results were being silently truncated.

The dbt SL GraphQL API pages at ~1024 rows. Arrow Flight, which the Python implementation used, streams the whole result — so this is a hazard the GraphQL port introduced rather than inherited, and the package doc actively hid it by asserting "pagination never engages". createQuery only sent a limit when one was given, and the tool never defaulted it, so the common case of a model omitting limit issued an unbounded query.

Live, against production data:

total_activities grouped by activity_project_id__organization_name, no limit
  before:  row_count 1024   <- the page boundary, not the answer
  after:   row_count 500    <- the ceiling the tool advertises

A truncated result reported as complete is the same class of failure this tool set exists to prevent — it is the row-count analogue of a wrong filter literal returning zero rows instead of an error. Fixed on both sides: Query now follows totalPages to the end, and an omitted or negative limit defaults to 500.

Also fixed

• similarityRatio did not reproduce difflib. The backwards j scan inverted difflib's tie-break, so among equal-length runs it kept the latest j instead of the earliest, stranding the rest of a against a shorter tail. 879 of 3490 ordered pairs of allowlisted metric names (25%) disagreed with real difflib. Since the whole point of hand-porting rather than using an edit distance was that suggestions would not shift in the port, this defeated the exercise.
• The poll loop had no bound of its own — stdio runs on a background context and the HTTP server sets only ReadHeaderTimeout, so a query stuck in RUNNING polled until the process exited. It also had the opposite problem: any transport error aborted immediately, so a single 502 mid-poll discarded a query still running upstream, which the Python route had retried. Now bounded, and tolerant of a few consecutive transport errors, while FAILED still aborts at once.
• FetchDimensionValues accepted an empty metric list. Verified live: FetchDimensions(ctx, nil) returns 295 dimensions, every one in the environment. Not reachable through the tool (the handler rejects empty metrics first), but the gate is documented as living in the client.
• Per-file package comments on all nine new files, per AGENTS.md.

On the test that let the difflib bug through

It used eight hand-picked pairs and passed both before and after the fix — every pair happened to be one where the tie-break does not bite. It is replaced by a sweep of the full domain against ratios generated by difflib itself, checked in as a fixture. I verified the replacement has teeth by running it against the old implementation (879/3490 fail) and the new one (0 fail). Worth stating plainly: the original test was the actual defect here, not just the code.

Verification

make check clean, go test ./... and go test -race pass. The live parity harness is green end to end (11/11 TestLive* against the real semantic layer), and the paging fix was confirmed through the real MCP server over stdio, not just against stubs.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/dbtsl/metadata.go:58

• The dimension metadata query omits queryableTimeGranularities, so get_dimensions can silently drop supported grains. The dbt reference client requests both fields and combines them (for example, its fixtures have queryableGranularities: ["day"] and queryableTimeGranularities: ["month"]); this implementation currently reports only the former as queryable_time_granularities. Request and decode both fields, then merge them when building DimensionInfo (and cover a grain present only in queryableTimeGranularities).

      queryableGranularities

internal/dbtsl/parity_live_test.go:82

• This parity check still passes when one or more allowlisted metrics disappear upstream; it only fails when the entire intersection is empty. That defeats the stated rename-detection purpose and allows a stale metric to remain accepted by ValidateMetrics even though queries for it fail. Make a non-empty missing list fail the test rather than only logging it.

	if len(missing) > 0 {
		t.Logf("allowlisted but not reachable upstream: %v", missing)
	}

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/dbtsl/dimensionvalues.go:134

• search is described as a plain substring, but % and _ are inserted into ILIKE without escaping and therefore remain SQL wildcards. A literal search such as 100% or _unknown can match unrelated values. Escape LIKE metacharacters as well as quotes and add an explicit ESCAPE clause supported by the Semantic Layer SQL dialect.

	if search != "" {
		args.Where = []string{fmt.Sprintf(
			"{{ Dimension('%s') }} ILIKE '%%%s%%'", dimension, escapeSQLLiteral(search),
		)}

internal/dbtsl/dimensionvalues.go:180

• This cannot reliably indicate truncation. Exactly limit total values is reported as truncated even when the result is complete; conversely, because NULLs are removed after the upstream limit is applied, a page containing NULL plus additional unseen values can produce fewer than limit values and be reported complete. Fetch an extra non-null row (for example, filter NULL and request limit+1) and derive Truncated before slicing the returned values to limit.

func newDimensionValues(dimension string, values []string, limit int) *DimensionValues {
	return &DimensionValues{
		Dimension:  dimension,
		Values:     values,
		ValueCount: len(values),
		Truncated:  len(values) >= limit,

internal/dbtsl/allowlist.go:274

• The fuzzy fallback does not preserve difflib.get_close_matches ranking for equal ratios. AllowedMetricNames starts in ascending order and this stable sort keeps ties ascending, while Python's nlargest over (score, candidate) ranks equal-score candidates in descending lexical order. Add the candidate name as a descending tie-break so the port does not reorder tied suggestions.

	sort.SliceStable(closest, func(i, j int) bool { return closest[i].ratio > closest[j].ratio })

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/dbtsl/parity_live_test.go:82

• This parity check still passes when most allowlisted metrics disappear upstream: missing is only logged, while the only assertion requires one reachable metric. A renamed or removed metric would therefore leave the production allowlist stale without failing the harness. Make the missing set fail the test.

	if len(missing) > 0 {
		t.Logf("allowlisted but not reachable upstream: %v", missing)
	}

internal/dbtsl/dbtsl_test.go:1053

• This regression test does not distinguish json.Number from the old float64 decoding: 4,239,559 is exactly representable, and encoding/json marshals that float as 4239559 without an exponent, so all current assertions also pass with UseNumber removed. Assert the decoded type (or use an integer above 2^53) so the test actually protects exact numeric decoding.

func TestParseQueryResultKeepsLargeIntegersExact(t *testing.T) {
	raw := `{"schema":{"fields":[{"name":"total_contributors","type":"integer"}],"primaryKey":[]},
	         "data":[{"total_contributors":4239559}]}`

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

internal/tools/semanticlayer.go:66

• This standalone query-tool description says it covers all events and maintainers, while only redirecting narrative questions to Lens. Because either semantic-layer tool can be enabled independently, callers that only see this schema will miss the exceptions documented by the explore tool and route event sponsorships or maintainer trends here. Include those exceptions in this description as well.

const querySemanticLayerDescription = `The LFX Insights Semantic Layer is the query and data-exploration tool for Linux Foundation data; this half runs the query. Covers contributions, memberships, events, education, maintainers and project health — and anything sliced by country or region. ALWAYS call explore_lfx_semantic_layer first unless you already have the exact metric, dimension and entity names — never guess or assemble one: a wrong name errors, and a wrong filter value returns no rows rather than an error, so confirm literals with get_dimension_values. Use query_lfx_lens for narrative or "why" questions that do not reduce to a metric.

internal/dbtsl/allowlist.go:139

• NoMetricsDetail renders this list as recommended recovery topics, so every failed metric search currently tells callers to try sponsorship. That conflicts with the tool descriptions and tests that assign event sponsorships to query_lfx_lens; remove it from the advertised fallback topics.

	"sponsorship",

internal/dbtsl/parity_live_test.go:82

• This parity check still passes when one or more allowlisted metrics disappear upstream, so an upstream rename will not actually fail the harness despite the test's stated purpose. Make missing metrics a test failure to preserve the allowlist/API parity guarantee.

	if len(missing) > 0 {
		t.Logf("allowlisted but not reachable upstream: %v", missing)
	}

README.md:293

• The new user-facing guidance is broader than the tool contract: event sponsorships and maintainer trends both reduce to metrics, but the semantic-layer descriptions explicitly route them to query_lfx_lens. Qualify this recommendation so users are not directed to the wrong backend.

Prefer these over `query_lfx_lens` for anything that reduces to a metric: the
answer is repeatable and auditable, where generated SQL is neither.

[josep-reyero]

Confirmed against the live API and fixed in 2065510. Good catch — this was worse in practice than on paper.

search "_"    -> 20 values: [Argentina Bahamas Bahrain Bangladesh Barbados ...]   # every country
search "%"    -> 20 values: [Angola Antarctica Antigua and Barbuda Austria ...]   # everything
search "viet" ->  1 value:  [Viet Nam]                                            # correct

This action exists to hand back exact literals, because a filter naming a real dimension with an unknown value returns zero rows rather than an error. Returning a list of values that did not match inverts that: it reads as though they did.

One correction to the suggested fix. ESCAPE is supported, but backslash is the wrong escape character — it is also the escape character of the SQL string literal, so it is consumed during string parsing before ILIKE ever sees it. I tried it first and the pattern '%\_%' ESCAPE '\\' matched Germany and Yemen, neither of which contains an underscore. Using !, which has no meaning inside a string literal, gives the right result: literal _ and % searches now return 0 rows and viet still returns Viet Nam.

Worth noting where the test for this landed. A stub server cannot catch it — it echoes whatever rows the test queued, so the ILIKE pattern is never evaluated. The unit test only pins the clause that gets built; the one that would actually have caught the bug is in the parity harness (TestLiveDimensionValuesTreatsSearchAsALiteral), where the real ILIKE runs and a literal _ search has to return nothing.

[Copilot]

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

internal/dbtsl/query.go:361

• The nested GroupByInput in an order clause also requires timeGranularity; grain is not the GraphQL field name. As written, ordering by metric_time__year (or another time grain) is rejected even if the corresponding group-by is fixed.

				groupBy["grain"] = grain

[josep-reyero]

Checked this against the deployed schema and it does not hold — grain is a real field and the queries run.

Introspecting GroupByInput on our environment:

name:            String          (NON_NULL)
timeGranularity: String          (SCALAR)
grain:           TimeGranularity (ENUM)
datePart:        DatePart        (ENUM)
alias:           String          (SCALAR)

Both exist. timeGranularity is a loose string, grain is the typed enum, and the enum is the better choice here precisely because the server validates it — a bad grain is rejected at the API rather than silently producing the wrong bucket.

The claim that this "makes GraphQL reject every grouped time-series query before execution" is testable, so I tested it. Raw mutation with grain: YEAR straight at the API returns a queryId and compiles, no error. End to end through this client:

--- PASS: TestLiveQueryWithTimeGrain (1.12s)
    columns: [METRIC_TIME__YEAR TOTAL_CONTRIBUTORS], rows: 5
      map[METRIC_TIME__YEAR:2030-01-01T00:00:00.000Z TOTAL_CONTRIBUTORS:1]
      ...

That test is in the parity harness and runs against the live semantic layer, so it is exactly the assertion this comment predicts would fail.

I think the reasoning is from the current Python SDK's serialization rather than our deployed GraphQL schema. Worth flagging that the two disagree in the other direction too: the published docs name the createQuery ordering argument order, while the deployed schema calls it orderBy — introspection caught that one during this port, and it would have broken every ordered query. Introspection against the live endpoint is the source of truth for this client, not the SDK or the docs.

No change made.