Skip to content

execute_sql trust receipt over-reports metrics: substring binding match on COUNT(*) yields false positives #144

Description

@sandeep-agami

Summary

assemble_receipt decides a metric was "used" by an answer via a raw substring containment test: it checks whether the metric's normalized binding SQL is contained in the normalized query SQL. Metrics whose binding is a short, ubiquitous expression — most notably the auto-generated <table>_count metrics, whose binding is COUNT(*) — therefore match almost any aggregate query. The execute_sql trust receipt's metrics array gets flooded with false positives.

Where

packages/agami-core/src/semantic_model/runtime.py, in assemble_receipt (metric loop, ~L1315–1330):

nsql = _norm_sql(sql)
for sa in org.subject_areas:
    for met in sa.metrics:
        binding = next((b for b in (met.bindings or {}).values()
                        if b and _norm_sql(b) in nsql), "")   # <-- substring containment
        if not binding:
            continue
        receipt["metrics"].append({ ... })

_norm_sql(b) in nsql is a plain substring test. COUNT(*) is a substring of essentially every aggregate query.

Repro

Any query containing COUNT(*) drags in every <table>_count metric. Example — an overall win-rate query:

SELECT COUNT(*) FILTER (WHERE is_won)::numeric
     / NULLIF(COUNT(*) FILTER (WHERE is_closed), 0) AS win_rate
FROM opportunities

This touches a single metric (win rate), but the returned receipt.metrics lists ~18: all 16 <table>_count metrics (binding COUNT(*), which is a substring of count(*) filter (where ...)), plus win rate and won opportunities (which genuinely match). A bare SELECT COUNT(*) FROM opportunities lights up all 16 count metrics on its own.

Observed receipt.metrics for the win-rate query above (names only): accounts_count, campaign_members_count, campaigns_count, cases_count, contacts_count, contracts_count, events_count, leads_count, opportunities_count, opportunity_contact_roles_count, opportunity_line_items_count, pricebook_entries_count, pricebooks_count, products_count, tasks_count, users_count, win rate, won opportunities.

Impact

  • The trust receipt's metric lineage is unreliable — it over-claims which governed metrics an answer used, on any query that happens to contain a COUNT(*).
  • The approve/change banner (tools.py instructs surfacing receipt.metrics whose review_state != "approved") shows spurious metrics, adding noise and eroding confidence in the receipt — the opposite of what a trust receipt is for.

Fix directions

  • Match on the parsed AST rather than a raw substring: require the binding to appear as a complete expression node (its own aggregate/expression), not any substring.
  • Exclude degenerate bindings (a bare COUNT(*) with no FILTER/predicate) from lineage matching, or require the binding to be more specific than a single ubiquitous token.
  • At minimum, require token / word-boundary alignment so count(*) does not match inside count(*) filter (where ...).

The docstring already states the intended contract ("a metric is 'used' when its binding SQL appears in the query"); the implementation under-constrains "appears."

Suggested regression test

  • A single-metric aggregate query (e.g. the win-rate query above) returns exactly the intended metric(s) in receipt.metrics.
  • SELECT COUNT(*) FROM <table> returns at most the one <table>_count metric, not every _count metric in the model.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions