← Back to PRs

#15999: fix: handle null/undefined in activity feed filters

by pamnlambert open 2026-02-14 05:04 View on GitHub →
app: web-ui stale size: S
## Summary Fixes activity feed crash when filtering by agent name, channel, chat type, label, or session ID with null/undefined values. ## Changes - Fixed 5 filter types in usage-helpers.ts - Pattern: Changed `?.toLowerCase().includes(x) ?? false` to `(?.toLowerCase() ?? "").includes(x)` - Added comprehensive test coverage (8 tests total, all passing) ## Testing ✅ All 8 tests passing in usage-helpers.node.test.ts ✅ Build successful ✅ Lint passed ## Root Cause Optional chaining (`?.`) returns `undefined` when property is null/undefined, causing `.includes()` to throw before the `?? false` fallback can apply. ## Solution Wrap optional chaining result with empty string fallback `(?? "")` before calling `.includes()`. Commit: 936ebfc <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR refactors 5 filter expressions in `usage-helpers.ts` from `?.toLowerCase().includes(x) ?? false` to `(?.toLowerCase() ?? "").includes(x)`, and adds 3 new test cases covering null/undefined values for `agentId`, `channel`, `chatType`, `label`, and `sessionId`. - **The code change is functionally a no-op.** The original optional chaining pattern (`?.`) already short-circuits the entire member access chain when the property is nullish — `.includes()` was never called on `undefined`. Both the old and new patterns produce identical results. - **The stated root cause is incorrect.** The PR description claims `.includes()` throws before `?? false` can apply, but optional chaining prevents `.includes()` from being called at all when the base is nullish. - **Type mismatch in tests:** The tests assign `null` to properties typed as `string | undefined` in `UsageSessionQueryTarget`. If `null` is a realistic runtime value (evidence suggests it is, e.g., `label?: string | null` in `sessions.ts`), the root fix should update the `UsageSessionQueryTarget` type definition to include `| null` for these fields. - **Tests are still valuable** as regression coverage, even if the original code was not buggy. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — the code change is a harmless refactor and the tests add useful coverage. - The refactored code is functionally equivalent to the original, so there is no risk of regression. The new tests add coverage for edge cases. The main concern is that the PR's stated root cause is incorrect (optional chaining already handled nullish values correctly), and the real type-safety issue (UsageSessionQueryTarget not including `| null`) is unaddressed. - The `UsageSessionQueryTarget` type definition in `ui/src/ui/usage-helpers.ts` should be updated to include `| null` for fields that can be null at runtime (agentId, channel, chatType, label, sessionId). <sub>Last reviewed commit: 936ebfc</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs