feat(dashboards): support constant values and render modes for filters

[alex-fedotyev]

Linear: HDX-4404.

Summary

• Adds two fields to DashboardFilterSchema: constant: boolean (locks the saved default; the viewer cannot clear it) and renderMode: 'editable' | 'readonly' | 'hidden' (controls how the chip appears in the filter bar).
• One dashboard can be cloned and re-pointed by saving a different default value per copy, instead of hand-coding the scope into every tile's WHERE.
• Filter editor UI presents a single "Visibility" select with three presets (Editable / Read-only / Hidden) that map to the orthogonal (constant, renderMode) pair. The schema still admits all combinations so MCP and external API callers can express future variants.
• When Visibility is Read-only or Hidden, the editor exposes a "Default value" TagsInput so the author can set the locked value directly. Editable filters keep using the existing chip + Save default flow unchanged.
• External API FilterInput / Filter OpenAPI schemas and the MCP mcpDashboardFilterSchema carry both new fields; openapi.json regenerated.
• Hook (useDashboardFilters) accepts savedFilterValues and overlays constant filter values on every read, regardless of URL state; setFilterValue is a no-op for constant expressions. Constant values are kept out of the URL on hydration so the lock doesn't leak into shared links, and handleSaveQuery preserves constant entries when saving so the editable Save default flow doesn't clobber them.
• Out of scope (tracked as follow-ups in the Linear issue): URL-parameterized constants (?scope=...), inline operator/values on the filter definition, customer-facing docs in clickhouse-docs.

Test plan

• Hook unit tests: constant value injection, setFilterValue no-op, sibling editable filters still work, getFilterQueriesForSource honors appliesToSourceIds for constant filters, hidden + constant resolves the saved value, missing saved value returns nothing.
• Zod schema tests: accepts constant + renderMode in every valid combination, rejects unknown renderMode, rejects null for either field.
• External API round-trip integration test: POST + GET + PUT with constant and renderMode set; absent fields stay absent on read; unknown renderMode returns 400.
• MCP hyperdx_save_dashboard round-trip integration test: same coverage as the external API test.
• UI smoke (Playwright + manual screenshots, light + dark):
  • Edit a filter, switch Visibility to Read-only, type a value into Default value, save. Verify the chip shows disabled with a lock icon and tiles include the locked value in WHERE.
  • Switch Visibility to Hidden. Verify the chip is removed from the bar; tiles still include the locked value.
  • Clear the Default value (TagsInput empty), save. Verify tiles are no longer scoped on that expression.
  • Hard-reload with an empty URL. Verify the locked value still applies (sourced from savedFilterValues).
  • Open a shared link with a stale value for the locked expression. Verify the locked value still wins.
• Predicted tier: review/tier-4 (touches packages/api/src/routers/external-api/v2/dashboards.ts, expected per the implementation plan).

Tagging as draft so the UI smoke can be done against this branch's build.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4163d2c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Minor
@hyperdx/api	Minor
@hyperdx/app	Minor
@hyperdx/otel-collector	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 30, 2026 5:23am
hyperdx-storybook	Ready	Preview, Comment	May 30, 2026 5:23am

[github-actions]

E2E Test Results

✅ All tests passed • 191 passed • 3 skipped • 1319s

Status	Count
✅ Passed	191
❌ Failed	0
⚠️ Flaky	4
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

• Critical-path files (2):
  • packages/api/src/routers/external-api/v2/dashboards.ts
  • packages/api/src/routers/external-api/v2/utils/dashboards.ts
• Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats

• Production files changed: 14
• Production lines changed: 1091 (+ 837 in test files, excluded from tier calculation)
• Branch: alex/HDX-4404-dashboard-filter-modes
• Author: alex-fedotyev

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

[github-actions]

<!-- deep-review -->

Deep Review

🔴 P0/P1 -- must fix

• packages/api/src/mcp/tools/dashboards/saveDashboard.ts:53 -- hyperdx_save_dashboard's MCP inputSchema exposes id/name/tiles/tags/containers/filters but not savedFilterValues, while mcpDashboardFilterSchema tells the agent that constant: true takes its locked value from dashboard.savedFilterValues; the safeParse payloads at :161 and :354 therefore never receive savedFilterValues, so a constant filter created via MCP locks to nothing and scopes no tile.
  • Fix: Add savedFilterValues: z.array(z.object({ type: z.enum(['lucene','sql']), condition: z.string() })).optional() to the tool inputSchema, thread it through to the create/update body schemas, and add an example pairing constant: true with a matching entry to mcpFiltersParam.
  • _agent-native

🟡 P2 -- recommended

• packages/app/src/DBDashboardPage.tsx:1445 -- savedFilterValues is keyed by filter expression, so when two filter definitions share an expression (one constant, one editable) getDefaultValuesForExpression seeds the editable form with the constant's locked value; if the author clears the picker and saves the editable, handleSaveFilter deletes the entry for that expression and silently breaks the sibling constant's lock.

  • Fix: Key savedFilterValues by filter id (and update the hook overlay + URL strip + save preservation to match), or reject sibling definitions where one is constant and one is editable on the same expression.
  • _{adversarial, correctness}

• packages/app/src/DashboardFiltersModal.tsx:304 -- DashboardFilterEditForm recomputes initialDefaultValues whenever the prop savedFilterValues changes and its useEffect([filter, initialDefaultValues, reset]) calls reset(...) on the change, so a background refetch that lands while the modal is open wipes any in-progress edit to the defaultValues field.

  • Fix: Snapshot initialDefaultValues once via a ref (or gate the reset on !formState.dirtyFields.defaultValues) so subsequent savedFilterValues arrivals don't clobber the author's typed values.
  • _{reliability, julik-frontend-races}

• packages/app/src/hooks/useDashboardFilters.tsx:102 -- inside the iteration, valuesForExistingFilters[expression] = match is keyed by raw expression, so two filter definitions sharing one expression with mixed constant/editable last-writer-wins: the editable's URL value overwrites the constant's locked value, breaking the lock guarantee for the in-scope tile.

  • Fix: Resolve values per-definition (e.g. key by filter.id and merge in getFilterQueriesForSource), or refuse to overwrite a key already populated by a constant entry.
  • _{adversarial, correctness}

• packages/api/openapi.json:2366 -- the new property schemas declare "default": false / "default": "editable" (mirrored in the swagger JSDoc at packages/api/src/routers/external-api/v2/dashboards.ts:1382), but the Zod schema is .optional() with no .default() and the new API test explicitly asserts the omitted fields return undefined; generated SDKs will surface a default the wire format never produces.

  • Fix: Drop the default keys from both property schemas in openapi.json and the JSDoc; describe the implied default in the description text instead.
  • _api-contract

• packages/app/src/DBDashboardPage.tsx:1669 -- the new constant-preservation merge in handleSaveQuery, the URL-strip block at :1581, and the savedFilterValues upsert in handleSaveFilter (:1445) are all non-trivial and entirely uncovered; future edits to any of the three inline implementations can silently regress the "Save default doesn't clobber constants" contract.

  • Fix: Extract each merge into a pure helper (mergeConstantFiltersForSave, stripConstantsFromUrl, upsertSavedDefault) and unit-test the matrix: constants present + URL collisions, no constants, constants without saved entries, bracket-notation expressions.
  • _{testing, maintainability}

• packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx:355 -- every new constant-filter test uses simple dot-notation expressions (environment, service.name); bracket-notation (SpanAttributes['k8s.pod.name']) flows through normalizeKey in the hook overlay, the URL strip, save preservation, and getDefaultValuesForExpression in the modal but has zero coverage.

  • Fix: Add a constant: true test with a bracket-notation expression and assert the locked value resolves through filterValues, blocks setFilterValue, and survives handleSaveQuery.
  • _{testing, correctness}

🔵 P3 nitpicks (12)

• packages/app/src/DBDashboardPage.tsx:1583 -- constantExpressions is rebuilt inline in the URL-hydration effect, again in handleSaveQuery, and a third time as a memo inside useDashboardFilters, each repeating parseKeyPath(...).join('.').

  • Fix: Promote normalizeExpression(k) and buildConstantExpressionSet(filters) into a shared module and call from all three sites.
  • _{maintainability, correctness}

• packages/common-utils/src/types.ts:1162 -- the new DashboardFilterRenderMode Zod enum is exported but every consumer in the app package uses the literal strings 'editable' / 'readonly' / 'hidden', including a hand-rolled FilterVisibility union and a getFilterVisibility parameter type.

  • Fix: Import the enum (or z.infer<> it into a type) and replace the magic strings so a future rename breaks the type-checker.
  • _{maintainability, kieran-typescript}

• packages/app/src/DashboardFiltersModal.tsx:87 -- applyFilterVisibility('editable') returns { constant: undefined, renderMode: undefined }; spreading that into the saved filter leaves the keys present (with undefined values) in memory, so structural equality against a JSON-roundtripped server copy sees a spurious difference even though the wire format is identical.

  • Fix: Return {} for the editable case so the keys are genuinely absent in memory.
  • _{kieran-typescript}

• packages/app/src/DashboardFiltersModal.tsx:412 -- the defaultsChanged comparison runs sanitizedDefaults.some((v, i) => v !== initialNormalized[i]), so reordering the same set fires a spurious save write.

  • Fix: Replace the positional compare with a set-equality check.

• packages/app/src/DashboardFiltersModal.tsx:195 -- NEVER_USED_RANGE is wrapped in useMemo with an empty deps array inside the component.

  • Fix: Hoist it to module scope as a plain const.

• packages/app/src/DashboardFiltersModal.tsx:188 -- the as DashboardFilter cast on filtersForQuery is redundant; the Pick<> type is already structurally assignable because the omitted fields are optional, so the cast just hides whether the assignment is sound.

  • Fix: Drop the cast (or widen the prop type) so the type relationship is explicit.

• packages/app/src/hooks/usePresetDashboardFilters.tsx -- the preset handleSaveFilter keeps its single-argument signature while the modal's onSaveFilter prop now accepts an optional { defaultValues } second arg; safe today because supportsConstantFilters=false on presets, but it silently drops the arg if that flag is ever flipped.

  • Fix: Widen the signature to (filter, _options?: { defaultValues?: string[] }) => void.

• packages/app/src/DBDashboardPage.tsx:170 -- the new parseQuery import uses a relative ./searchFilters path while every other call site in the package uses the @/searchFilters alias.

  • Fix: Switch to the alias for consistency.

• packages/app/src/DBDashboardPage.tsx:1622 -- dashboard?.filters was added to the hydration effect's deps but no body code closes over it; the initializedDashboard.current === dashboard.id guard already protects against re-runs, so the extra dep only widens the re-execution surface.

  • Fix: Drop dashboard?.filters from the dep array and add a one-line eslint-disable explaining why.
  • _{reliability, julik-frontend-races}

• packages/app/src/DBDashboardPage.tsx:1470 -- handleRemoveFilter deletes a filter from dashboard.filters but leaves any savedFilterValues entry keyed by that filter's expression; recreating a filter on the same expression later silently re-locks to the orphaned value.

  • Fix: Strip the matching savedFilterValues entry alongside the filter delete.
  • _correctness

• packages/common-utils/src/types.ts:1183 -- the Zod schema admits renderMode: 'hidden' (or 'readonly') with constant: false; that combination produces a chip-less filter that never applies (no constant overlay) and is reachable from external API and MCP callers.

  • Fix: Either add a Zod refine rejecting non-editable renderMode without constant: true, or document the combo as a no-op in the field description.
  • _correctness

• packages/api/src/routers/external-api/__tests__/dashboards.test.ts:961 -- the HDX-4404 round-trip test uses toMatchObject on the filter payload, which lets unexpected extra fields slip through silently.

  • Fix: Use toEqual on the filter objects (or assert the full expected key set) for both POST and PUT responses.
  • _testing

---

Reviewers (11): correctness, testing, maintainability, project-standards, security, learnings-researcher, api-contract, reliability, adversarial, kieran-typescript, julik-frontend-races, agent-native.

Testing gaps:

• Page-level handleSaveQuery, handleSaveFilter, and URL-strip-on-hydration paths in DBDashboardPage.tsx have no direct coverage.
• Bracket-notation expressions are unverified across the hook overlay, URL strip, and save preservation paths.
• No component test exercises DashboardFiltersModal's Visibility select or the default-value picker (the filter-visibility-select / filter-default-values-input testids are unused).
• No test asserts that DashboardFilters actually omits chips for renderMode: 'hidden' from the DOM.
• No test covers two filter definitions sharing an expression with mixed constant/editable flags.
• No Playwright/E2E coverage for the locked-filter authoring flow.

[alex-fedotyev]

Pushed 21880e4 with the deep-review fix-pack plus the e2e shard-2 root cause. Summary:

E2E shard 2 (3 failing tests)

Three pre-existing dashboard tests (should create and populate filters, should scope a filter to a specific source via "Applies to sources", should save and restore query and filter values) failed because visibleFilters in DashboardFilters.tsx was a new array reference every render, so useDashboardFilterValues's useQueries churned and the chip dropdown stayed disabled while the values query never settled. Memoizing the array fixes the regression. The Playwright error-context.md snapshot confirmed the failure mode (both filter chips rendered [disabled] even though neither had constant / renderMode set, and the underlying table tile had loaded its data).

Deep review

• P0/P1 saveDashboard.ts:53 MCP inputSchema missing savedFilterValues. Fixed in 21880e4 — threaded savedFilterValues through the tool input, the create/update helpers, and added an example pairing constant: true with a savedFilterValues entry in mcpFiltersParam.
• P2 DashboardFiltersModal.tsx:304 reset-wipes-edits race. Fixed in 21880e4 — snapshot initialDefaultValues via state keyed on filter.id; a background savedFilterValues refetch no longer fires the reset.
• P2 useDashboardFilters.tsx:102 + DBDashboardPage.tsx:1445 sibling-expression collision. Fixed in 21880e4 — added a Zod refinement at the boundary (refineDashboardFiltersConstantSiblings) that rejects two filter definitions sharing a normalized expression where one has constant: true and another doesn't. The id-keyed wire-format refactor (the other option you suggested) is intentionally out of scope; I'll file a follow-up for that since it's a wire-format change.
• P2 openapi.json:2366 defaults declaration. Fixed in 21880e4 — dropped "default": false / "default": "editable" from both property schemas and the JSDoc; implied default text moved into the description.
• P2 DBDashboardPage.tsx:1669 page-level merge helpers uncovered. Fixed in 21880e4 — extracted normalizeExpression, buildConstantExpressionSet, stripConstantsFromUrl, mergeConstantFiltersForSave, upsertSavedDefault, removeSavedDefaultForExpression into dashboardFilterUtils.ts, with the matrix (constants + URL collisions, no constants, constants without saved entries, bracket-notation) covered by a new unit test.
• P2 useDashboardFilters.test.tsx:355 bracket-notation untested. Fixed in 21880e4 — added a constant: true test with SpanAttributes['k8s.pod.name'] covering hook overlay, setFilterValue no-op, and getFilterQueriesForSource.

P3 nitpicks all addressed in the same commit:

• DBDashboardPage.tsx:1583 constantExpressions rebuilt three times — now one helper.
• types.ts:1162 enum unused — FilterVisibility now z.infer<typeof DashboardFilterRenderMode>.
• DashboardFiltersModal.tsx:87 applyFilterVisibility('editable') returns {} instead of { constant: undefined, renderMode: undefined }.
• DashboardFiltersModal.tsx:412 set equality instead of positional compare.
• DashboardFiltersModal.tsx:195 NEVER_USED_RANGE hoisted to module scope.
• DashboardFiltersModal.tsx:188 as DashboardFilter cast dropped.
• usePresetDashboardFilters.tsx widened to accept _options?: { defaultValues?: string[] }.
• DBDashboardPage.tsx:170 parseQuery import uses the @/searchFilters alias.
• DBDashboardPage.tsx:1622 dashboard?.filters dropped from deps (with an eslint-disable explaining why).
• DBDashboardPage.tsx:1470 handleRemoveFilter strips the matching savedFilterValues entry.
• types.ts:1183 renderMode: 'readonly' | 'hidden' without constant: true rejected by refineDashboardFilterCoherence.
• dashboards.test.ts:961 toMatchObject swapped to toEqual so unexpected fields fail the assertion.

make ci-lint + 45 touched unit tests pass locally. Waiting on CI for the e2e shard-2 retry.

[alex-fedotyev]

Pushed 73d370d with two CI fixes after digging into the shard 2 trace:

Integration test: dashboards.test.ts:965 was asserting whereLanguage: 'sql' on filters whose input payload omits whereLanguage. The Zod schema is .optional() with no default, so the wire format is undefined-in / undefined-out. Dropped the field from three filter expects in the round-trip test plus the PUT-flip expect; added a comment so the omission is deliberate.

E2E shard 2: trace.network shows a 500 on clickhouse-proxy at the moment of filter editing:

Code: 457. DB::Exception: Empty Identifier part after parameter `HYPERDX_PARAM_0` substitution.

That HYPERDX_PARAM_0 is the hash of an empty string. The SQL body is DESCRIBE {db}.{} with an empty Identifier substitution. The FilterDefaultValueSelect I added inside the modal fires useDashboardFilterValues while editing, and for a metric source with no metricType picked yet, getMetricTableName(source, undefined) resolves to source.from.tableName, which is empty on metric sources. That's what put the request together and got a 500. The 500 left the chip dropdowns disabled long enough to time out the option clicks.

Gated filterForValueQuery on tableName resolving non-empty so the picker stays idle until the form is configured enough to name a real table.

Local make ci-lint passes, make ci-unit passes (1782 / 1782; one ENOMEM in TraceRedirectPage.test.tsx that's environment, unrelated). Waiting on CI shard 2.

[github-actions]

Deep Review

🔴 P0/P1 -- must fix

• packages/app/src/DashboardFiltersModal.tsx:85 -- applyFilterVisibility('editable') returns {}, so when an author flips a saved readonly or hidden filter back to "Editable" the form's rest spread carries the original constant: true and renderMode through to the persisted filter, and the hook continues to treat the filter as locked while the visibility UI claims it is editable.

  • Fix: Return { constant: undefined, renderMode: undefined } from the editable branch, or destructure constant and renderMode out of rest alongside visibility and defaultValues so the spread of visibilityFields is authoritative.
  • _{correctness, maintainability, kieran-typescript, adversarial}

• packages/app/src/DashboardFiltersModal.tsx:446 -- the editor's filterForValueQuery memo depends on useWatch of expression, where, and whereLanguage, all backed by CodeMirror controllers that fire on every keystroke, so useDashboardFilterValues issues a new ClickHouse autocomplete request per character and frequently with partial WHERE strings that the proxy rejects server-side.

  • Fix: Debounce watchedExpression / watchedWhere / watchedWhereLanguage (e.g. useDebouncedValue from @mantine/hooks at ~300ms) before building filterForValueQuery, and only flip queryReady once the debounced snapshot is stable.
  • _performance

🟡 P2 -- recommended

• packages/app/src/DBDashboardPage.tsx:1580 + packages/app/src/hooks/useDashboardFilters.tsx:52 -- stripConstantsFromUrl runs only in the !hasFiltersInUrl hydration branch, so a stale shared URL keeps its value for an expression that is now constant: true; the hook ignores it on display, but the next setFilterValue call for any other filter re-parses the entire URL state via parseQuery and re-emits the constant entry via filtersToQuery, re-publishing the locked scope back into the URL and any subsequent share.

  • Fix: Run stripConstantsFromUrl on every load when constantExpressions is non-empty (drop the !hasFiltersInUrl gate for that branch) and strip constant-expression keys inside setFilterValue before filtersToQuery, so the hook acts as both an input guard and an output scrubber.
  • _adversarial

• packages/api/src/utils/zod.ts:160 + packages/api/openapi.json -- externalDashboardSavedFilterValueSchema.type accepts 'sql' | 'lucene' and defaults to 'sql', but the OpenAPI SavedFilterValue.type enum lists only [sql] and the UI's upsertSavedDefault routes through filtersToQuery which writes type: 'lucene'; an external caller that follows the spec sends a SQL-shaped condition that parseQuery may not key under the constant filter's expression, so the lock has no runtime effect.

  • Fix: Widen the OpenAPI SavedFilterValue.type enum to [sql, lucene], drop the "only sql is supported" copy, and either change the Zod default to 'lucene' to match the UI or explicitly document that constant-filter round-trips require type: 'lucene'.
  • _{agent-native, api-contract}

• packages/common-utils/src/types.ts:1270 -- refineDashboardFiltersConstantSiblings re-implements bracket→dot normalization with a global regex while the runtime overlay (useDashboardFilters via parseKeyPath) only strips a single bracket segment per call; on nested expressions like SpanAttributes['k8s']['pod'] the two paths produce different keys, so the schema can group as siblings two filters the hook treats as distinct (or vice versa).

  • Fix: Import parseKeyPath from ./core/metadata in types.ts -- it lives in the same package and is already imported by filters.ts, so there is no boundary cross -- or move the normalization into a leaf module both files can call.
  • _{kieran-typescript, maintainability}

• packages/common-utils/src/__tests__/dashboardFilter.test.ts -- the two new Zod refinements (refineDashboardFilterCoherence rejecting readonly/hidden without constant: true, and refineDashboardFiltersConstantSiblings rejecting mixed constant + editable on the same expression) ship without direct unit tests, so a future change weakening either to a no-op would not fail CI.

  • Fix: Add unit tests against both refinement helpers covering each rejection branch and at least one passing combination, including a bracket-vs-dot normalization case for the sibling check.
  • _testing

• packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx -- the cache-pollution fix moving filterIds derivation out of queryFn has no regression test, so a future refactor that puts it back would silently re-introduce the modal-'new'-ID poisoning the chip's filterValuesById lookup.

  • Fix: Render the hook once with a filter whose id is 'new' for some (source, expression, dateRange, where) tuple, then render again with the same tuple but a real UUID, and assert the returned Map is keyed by the real UUID even when the underlying call comes from the cache.
  • _testing

• packages/api/src/mcp/__tests__/dashboards.test.ts -- the MCP round-trip tests never set the new top-level savedFilterValues parameter even though it is the only way an MCP caller can supply the locked value paired with constant: true, so a regression that drops it on the wire would go undetected.

  • Fix: Add a round-trip test that creates a dashboard with constant: true + a paired savedFilterValues entry, reads it back, and asserts both arrays are intact using toEqual (not toMatchObject).
  • _testing

• packages/app/src/DashboardFiltersModal.tsx -- the modal grew ~390 net lines adding the Visibility select, FilterDefaultValueSelect, getFilterVisibility/applyFilterVisibility, initialDefaultValues snapshot, defaultsChanged set-equality, sanitization, and the "No default value set" alert, but there is no component test for the editor and no e2e selector exercises filter-visibility-select or filter-default-values-input.

  • Fix: Add a component test that mounts the editor, switches Visibility through all three presets, asserts onSave is called with the correct (constant, renderMode) pair plus options.defaultValues, and explicitly covers the readonly→editable round-trip (which catches the P1 above).
  • _testing

• packages/app/src/DBDashboardPage.tsx:1442 -- the four new wiring points (handleSaveFilter routing options.defaultValues through upsertSavedDefault, handleRemoveFilter calling removeSavedDefaultForExpression, handleSaveQuery calling mergeConstantFiltersForSave, and the hydration effect calling stripConstantsFromUrl under a disabled exhaustive-deps lint) ship without tests, and the eslint-disable leaves future refactors free to break the initializedDashboard.current === dashboard.id guard.

  • Fix: Extract the page-level handlers into thin testable units or add component-level tests asserting each behavior, including that a stale dashboard.id switch does not re-fire constant stripping.
  • _testing

• packages/app/tests/e2e/page-objects/DashboardPage.ts -- the +2 lines on the page object only fix an unrelated click sequence; the new Visibility selector, default-value input, locked-chip lock icon, and "No default value set" alert have no Playwright coverage even though the PR test plan itemizes each end-to-end scenario.

  • Fix: Add an e2e spec that creates a filter, picks Visibility=Read-only, sets a default value, saves, reloads, and asserts the chip renders with the lock icon, is disabled, and scopes a tile's WHERE clause to the locked value.
  • _testing

• packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx:391 -- the "saved value wins over URL value" test verifies the overlay but does not assert the URL-encountered constant expression is absent from ignoredFilterExpressions, so a regression that surfaces a stale "ignored filter" banner to viewers of a cloneable template would not be caught.

  • Fix: Add expect(result.current.ignoredFilterExpressions).toEqual([]) plus an analogous assertion that getFilterQueriesForSource returns empty when a constant has no saved value.
  • _testing

🔵 P3 nitpicks (12)

• packages/app/src/hooks/useDashboardFilters.tsx:101 -- pre-existing dashboards with mixed constant: true + editable siblings on the same expression (saved before the refinement or via a non-v2 path) fall through last-iteration-wins in valuesForExistingFilters; the editable sibling's URL value silently overrides the constant sibling's locked value while setFilterValue remains blocked, so the chip shows a stale URL value with a lock icon and the locked scope leaks.

  • Fix: Aggregate by normalized expression in the overlay loop -- if any sibling sets constant: true, use savedMatch; otherwise use urlMatch.

• packages/app/src/DBDashboardPage.tsx:1465 -- two constant: true siblings on the same expression are schema-legal when appliesToSourceIds differ, but handleRemoveFilter calls removeSavedDefaultForExpression which strips the only saved entry; the surviving sibling now renders with the lock icon but no value and no WHERE scoping.

  • Fix: Before pruning the saved entry, scan the remaining dashboard.filters for the same normalized expression and skip the prune if any sibling still references it.

• packages/common-utils/src/types.ts:1185 -- DashboardFilterSchema admits constant: true without requiring a paired savedFilterValues entry; the modal shows the IconLock Alert but MCP/REST callers see no error and ship a filter that consumes a slot, applies no WHERE, and is invisible when renderMode === 'hidden'.

  • Fix: Add a dashboard-level refinement flagging constant: true without a matching savedFilterValues entry, or downgrade to a superRefine warning that surfaces in the API response.

• packages/api/src/mcp/tools/dashboards/schemas.ts:724 -- mcpDashboardFilterSchema exposes constant and renderMode as independently optional with no .superRefine(refineDashboardFilterCoherence), so the LLM tool schema does not encode the rule and callers learn it only via a runtime body rejection.

  • Fix: Apply .superRefine(refineDashboardFilterCoherence) directly on mcpDashboardFilterSchema.

• packages/api/src/mcp/tools/dashboards/schemas.ts:732 + packages/api/src/routers/external-api/v2/dashboards.ts:1373 -- the sibling-agreement rule (two filters on the same expression must agree on constant) is enforced server-side but never surfaced in the MCP mcpFiltersParam.describe or the OpenAPI FilterInput.constant description, so callers hit a generic Zod error rather than a domain hint.

  • Fix: Add one sentence to both describe blocks explaining the constraint.

• packages/api/src/mcp/tools/dashboards/saveDashboard.ts:67 -- the UI strips savedFilterValues entries on filter delete, but the REST PUT and MCP update never auto-prune orphans and the describe block does not document the caller's responsibility.

  • Fix: Add a one-line note to the savedFilterValues describe (MCP + OpenAPI) that callers should drop entries for expressions they remove, or auto-prune server-side on update.

• packages/app/src/DashboardFiltersModal.tsx:280 -- FilterEditFormValues extends DashboardFilter plus a destructure of synthetic visibility and defaultValues at submit relies on naming discipline; a future synthetic field added without updating the destructure leaks into the persisted DashboardFilter, and the structural extend relation means TypeScript will not flag it.

  • Fix: Compose rather than extend (type FilterEditFormValues = { definition: DashboardFilter; visibility; defaultValues }) or declare a UI_ONLY_FIELDS const and derive both the omit and the type from it.
  • _{maintainability, kieran-typescript}

• packages/app/src/DashboardFilters.tsx:33 -- isReadOnly = renderMode === 'readonly' || !!constant covers the hidden + constant combo only because hidden filters are excluded one level up at visibleFilters; the name conflates "chip is rendered locked" with "chip is locked".

  • Fix: Rename to isLocked with a one-line comment noting the upstream visibleFilters exclusion.

• packages/app/src/DashboardFiltersModal.tsx:106 -- NEVER_USED_RANGE = [new Date(0), new Date(0)] relies on filtersForQuery: [] to keep the query from firing; a future refactor that gates the hook on dateRange[0] < dateRange[1] or fires a default-options call would silently issue a query against the epoch.

  • Fix: Make dateRange non-optional in the hook and gate the call site, or add enabled: !!dateRange inside the hook.

• packages/app/src/DashboardFiltersModal.tsx:78 -- applyFilterVisibility's case 'editable': default: masks future enum additions from exhaustiveness checking; a new DashboardFilterRenderMode variant would silently land in the editable branch.

  • Fix: Split into explicit case 'editable': and default: arms, with the default calling assertNever(visibility).

• packages/api/src/routers/external-api/v2/utils/dashboards.ts:1162 -- the double-cast data.filters as unknown as DashboardFilter[] claims source: string on the post-parse shape even though the external schema produced sourceId and omitted source; today only expression/constant are read so there is no runtime bug, but the cast documents the boundary incorrectly.

  • Fix: Narrow to Pick<DashboardFilter, 'expression' | 'constant'>[] via a map step (and update refineDashboardFiltersConstantSiblings's signature), or assert through the Pick instead of the full type.
  • _{kieran-typescript, project-standards}

• packages/api/src/mcp/__tests__/dashboards.test.ts:1391 -- the new filter assertions still use toMatchObject while the external-API test was explicitly upgraded to toEqual with a note that toMatchObject "lets unexpected fields slip through silently"; MCP-side schema drift can go unnoticed.

  • Fix: Switch the MCP filter round-trip assertion to toEqual on the full filter object.

Pre-existing

• packages/app/src/DashboardFiltersModal.tsx -- already 542 lines at base SHA and now 934, well past the repo-standard 300-line cap; this PR adds ~390 lines rather than splitting the editor surface into a sibling module. Follow-up: extract FilterDefaultValueSelect, the visibility helpers, and the locked-default Alert into sibling files. _{project-standards}

---

Reviewers (10): correctness, testing, maintainability, project-standards, agent-native-reviewer, learnings-researcher, api-contract, kieran-typescript, performance, adversarial.

Testing gaps:

• The "readonly→editable" Visibility toggle on an existing filter (the P1 above) is uncovered on every surface.
• The cache-pollution regression scenario, both Zod refinements, the MCP savedFilterValues round-trip, the new DashboardFiltersModal UI fields, the DBDashboardPage save/remove/hydration handlers, and the e2e Visibility / Default-value flow are itemized as P2s above.
• No contract test asserts that openapi.json and externalDashboardSavedFilterValueSchema agree on type enum and condition length.
• No test exercises setFilterValue when the URL already carries an entry for an expression that is now constant -- the rebuild should drop it, not re-emit it (drives the URL-leak P2 above).