Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
[Medium impact] The PR appears safe to merge. SummarySearch-tile cells and row side panels can update the ad hoc filters that apply to their source.
|
342f2df to
36b05ac
Compare
🦋 Changeset detectedLatest commit: daf6b09 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
Duplicate code analysisNo new duplicate code introduced by this PR. jscpd compared this branch against |
E2E Test Results✅ All tests passed • 434 passed • 1 skipped • 1312s
Tests ran across 4 shards in parallel. |
36b05ac to
d90319b
Compare
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
| ); | ||
|
|
||
| // `source` keeps the handler off rows from other sources, e.g. a linked trace | ||
| const searchRowContext = useMemo<RowSidePanelContextProps | undefined>( |
There was a problem hiding this comment.
🟠 major — Side panel's surrounding-context table writes unshaped cells (DateTime, Map, aliases) into ad hoc filters
searchRowContext reaches every table under the tile's RowSidePanelContext, not just the tile's own table. DBRowSidePanel passes it down to ContextSubpanel (components/ContextSidePanel.tsx:313), which renders a separate DBSqlRowTable with the source's default select and without isAdhocFilterTarget. That table skips getAdhocCellCondition. Clicking include on its Timestamp cell adds Timestamp = '2026-…Z'. Per isAdhocComparableDataType, that never equals toString(Timestamp), so every tile on the source goes empty. A Map cell or an aliased column also produces a condition that can't match or that refers to a missing column. Fix: put the shaping in the context itself, e.g. a flag on RowSidePanelContextProps that DBRowTableFieldWithPopover reads, instead of a prop on one table. Or drop onPropertyAddClick for nested tables.
Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond
| const searchRowContext = useMemo<RowSidePanelContextProps | undefined>( | ||
| () => | ||
| onFilter && source && (isLogSource(source) || isTraceSource(source)) | ||
| ? { |
There was a problem hiding this comment.
🔵 minor — Side panel "Add to Filters" removes an existing ad hoc condition while the toast says "Added"
DBRowJsonViewer.tsx:553-563 calls onPropertyAddClick with no action and then always shows Added "key = value" to filters. Here, no action is mapped to include. mergeAdhocConditions then toggles the condition off when it already exists, e.g. after a cell click already included ServiceName = frontend. The user sees "Added" while the condition disappears. Fix: map a missing action to a non-toggling add (skip entries already applied), or only toggle when the action comes from the cell popover.
Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond
PR Review
3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor 1 posted as inline comment(s) on the changed lines. 2 listed below. Findings outside the changed lines2 minor
Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides. |
Deep Review✅ No critical issues found. No P0/P1 defects are introduced by this diff. All five prior Greptile P1s (DateTime, structured-cell, alias-quoting, and session-source value/format mismatches) are verified resolved in the current code, and the clicked-value path is confirmed SQL-injection-safe (values flow through 🟡 P2 -- recommended
🔵 P3 nitpicks (4)
Pre-existing (not introduced by this diff)
Agent-Native Gap (advisory)
Reviewers (9): security, testing, maintainability, project-standards, api-contract, kieran-typescript, previous-comments, agent-native, learnings-researcher. The correctness and adversarial reviewers did not return before synthesis; their scope (value/format round-tripping, toggle/undo logic, filter targeting, alias/column resolution) was independently verified during this review and cross-covered by the previous-comments reviewer, which re-checked and confirmed resolution of the prior correctness bugs. Testing gaps:
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 |
3152baf to
b2041c8
Compare
b2041c8 to
be7f0f8
Compare
be7f0f8 to
3fac930
Compare
Summary
Support click-to-filter for search tiles on dashboards with adhoc filters.
The existing search table's click-to-filter functionality is re-used. The sidebar add filter button works as well.
Screenshots or video
Screen.Recording.2026-10-16.at.8.40.22.AM.mov
How to test on Vercel preview
References