feat(llma): Conversation title on session table (including title logic fixes)#61342
Open
mp-hog wants to merge 9 commits into
Open
feat(llma): Conversation title on session table (including title logic fixes)#61342mp-hog wants to merge 9 commits into
mp-hog wants to merge 9 commits into
Conversation
Contributor
|
Size Change: -2.29 kB (0%) Total Size: 81.9 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
products/ai_observability/frontend/AIObservabilitySessionScene.tsx:127-130
**Side effect called during render**
`ensureSessionTitleLoaded` is a Kea action dispatch that schedules a `setTimeout` and modifies external store state. Calling it conditionally inside the render function body breaks React's rule that renders must be pure. `SessionColumn` (the sibling component added in the same PR) correctly wraps the equivalent call in a `useEffect` — this site should do the same. In React's concurrent/strict modes the render function can be invoked more than once for a single logical update; the listener's `loadingSessionIds` guard prevents a double fetch, but the action dispatch itself during render is still a violation that future React versions may warn about or mishandle.
### Issue 2 of 2
frontend/src/lib/utils.test.ts:1100-1119
The `chunk` tests follow a uniform `(arr, size) → expected` shape and are better expressed as a single parameterized case, consistent with the team's preference for `it.each` tests.
```suggestion
describe('chunk', () => {
it.each([
{ desc: 'splits into evenly-sized chunks', arr: [1, 2, 3, 4], size: 2, expected: [[1, 2], [3, 4]] },
{
desc: 'leaves a smaller final chunk when not evenly divisible',
arr: [1, 2, 3, 4, 5],
size: 2,
expected: [[1, 2], [3, 4], [5]],
},
{ desc: 'returns one chunk when size exceeds length', arr: [1, 2], size: 10, expected: [[1, 2]] },
{ desc: 'returns an empty array for an empty input', arr: [], size: 3, expected: [] },
])('$desc', ({ arr, size, expected }) => {
expect(chunk(arr, size)).toEqual(expected)
})
})
```
Reviews (1): Last reviewed commit: "Merge branch 'master' into llma/conversa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Sessions list showed only session id. Even though this PR shows only a truncated user message, it's still more helpful to humans rather than a random ID.
Additionally, this PR improves the logic for title inference implemented on the details page in #60667
Changes
<system_reminder>chunkhelper intolib/utils.How did you test this code?
sessionTitle.test.tsandllmSessionTitleLazyLoaderLogic.test.ts$mcp_intentsignal, relying on the existing session-id bloom-filter index) were validated against production🤖 Agent context
Authored with Claude Code (agent-assisted, human-reviewed). Title is lazy-loaded per row because the message payloads aren't in the list's SQL once a team is on the
ai_eventstable; the per-session lookup needs no time bound because session id is bloom-filter indexed.