[knudtty]

Summary

Adds dependent fields for adhoc filters. This lays the groundwork to support source filters.

Screenshots or video

Notice the only value fetched when scoped to ServiceName = 'hdx-oss-dev-app' is the one scoped to the app service.

export-1793985985388.mp4

References

• Linear Issue: Related to HDX-5739

[github-actions]

🟡 Tier 3 — Standard

Introduces new logic, modifies core functionality, or touches areas with non-trivial risk.

Why this tier:

• Diff size: 468 production lines changed (Tier 2 max: < 250)
• Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Full human review — logic, architecture, edge cases.
SLA: First-pass feedback within 1 business day.

Stats

• Production files changed: 15
• Production lines changed: 468 (+ 794 in test files, excluded from tier calculation)
• Branch: aaron/adhoc-filter-values-condition
• Author: knudtty

To override this classification, remove the review/tier-3 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

2 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 2 minor

2 posted as inline comment(s) on the changed lines.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

E2E Test Results

✅ All tests passed • 463 passed • 1 skipped • 1309s

Status	Count
✅ Passed	463
❌ Failed	0
⚠️ Flaky	3
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

Scope: PR #3585 — dependent where condition for ad hoc dashboard filters (SQL sources only). Base 5b96dcb, 15 production files + tests across packages/app and packages/common-utils.

Intent: Narrow the keys/values suggested in an ad hoc filter's condition editor to rows matching an author-defined where (which may reference other dashboard variables), threading a rendered conditionSql through Metadata.getMapKeys/getAllFields with a TTL'd, condition-scoped cache key, and disabling the filter when referenced variables/required filters are unsatisfied.

✅ No critical issues found. No P0/P1 defects were identified by the reviewers that completed or by direct diff analysis. The UNSAFE_RAW_SQL path for the new where reuses the existing renderWhereExpressionStr mechanism already used for queried-filter and chart where clauses — it does not widen the trust boundary (author-authored condition + viewer variable values expanded via the same resolveFilterValuesWhere, run under the same connectionId). Schema additions are optional and backward-compatible for reading existing persisted dashboards.

🟡 P2 — recommended

• packages/common-utils/src/core/metadata.ts:1133 — When conditionSql is set, the text-index and rollup fast paths are skipped (!conditionSql guards), so every Map column on every SQL source falls through to a row-capped, time-bounded source scan whose results can be silently incomplete on large tables.
  • Fix: Validate scan cost under dashboards with several SQL sources and multiple Map columns, and signal truncated key/value suggestions rather than caching a partial list.
  • _performance

🔵 P3 nitpicks (4)

• packages/common-utils/src/core/metadata.ts:1952 — In getAllFields, the condition narrows Map-column keys (threaded to getMapKeys as conditionSql) but is not passed to getJSONKeys, so JSON-column key suggestions ignore the where.
  • Fix: Thread conditionSql to getJSONKeys as well, or document that condition scoping is Map-column only.
• packages/common-utils/src/types.ts:2425 — where/whereLanguage are added to AdhocDashboardFilterSchema unconditionally, so a PromQL ad hoc filter can persist a where that is never read or executed despite the "SQL sources only" comment.
  • Fix: Add a superRefine rejecting where on non-sql ad hoc filters, or document that a stored where on a PromQL ad hoc filter is inert.
• packages/common-utils/src/types.ts:2425 — Adding fields to the .strict() AdhocDashboardFilterSchema creates a write-new/read-old forward incompatibility: a dashboard saved with these fields fails to parse on an older client.
  • Fix: Confirm this matches the project's dashboard-schema versioning policy, or relax strictness for forward-compatible reads.
• packages/common-utils/src/core/metadata.ts:3438 — Exported FieldsCondition = Pick<BuilderChartConfig, ...> structurally couples the public type to BuilderChartConfig field names, so renaming any picked field silently changes the exported contract.
  • Fix: Accept the coupling deliberately or insulate it behind a named interface.

---

Reviewers (4 completed of 14 dispatched): security, performance, api-contract, previous-comments. Correctness, adversarial, reliability, testing, maintainability, kieran-typescript, frontend-races, project-standards, agent-native, and learnings-researcher were dispatched but had not returned when synthesis was required; correctness/adversarial coverage above is backfilled from direct orchestrator diff analysis and should be treated as partial.

Testing gaps:

• No test for conditionSql cache-key scoping (hit/miss across distinct conditions) or for the !conditionSql fast-path skips.
• useMultipleAllFields conditions is positionally coupled to tableConnections via conditions?.[i]; no test covers length/order mismatch.
• No test deserializes a pre-existing ad hoc filter JSON (without where) through AdhocDashboardFilterSchema to lock backward-compat, nor asserts where is excluded from broadcast/applied output.
• No test asserts a PromQL ad hoc filter's stored where is never rendered into a ClickHouse query.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}