[feat] Order and search the session list by last activity (2/12) - #5681
[feat] Order and search the session list by last activity (2/12)#5681ardaerzin wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughSession queries now support case-insensitive title search, cursor pagination, updated activity ordering, and latest-turn reference enrichment. Backend DTOs, services, DAOs, API models, frontend request types, schemas, and tests were updated. ChangesSession query enhancements
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionsRouter
participant SessionsService
participant SessionStreamsDAO
participant SessionTurnsService
participant SessionTurnsDAO
Client->>SessionsRouter: Query sessions with search and pagination
SessionsRouter->>SessionsService: Build SessionQuery
SessionsService->>SessionStreamsDAO: Fetch filtered, ordered streams
SessionStreamsDAO-->>SessionsService: Return session streams
SessionsService->>SessionTurnsService: Fetch latest turn per session
SessionTurnsService->>SessionTurnsDAO: Query highest-index turns
SessionTurnsDAO-->>SessionTurnsService: Return references by session ID
SessionsService-->>Client: Return SessionListItem response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
api/oss/tests/pytest/unit/sessions/test_query_sessions_search.py (1)
30-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated dummy-engine test doubles into a shared helper. Both new test files define identical
_DummyScalars,_DummyResult,_DummySession, and_DummySessionContextclasses to intercept the compiled SQLAlchemy statement. The shared root cause is the lack of a common test helper for this statement-compilation pattern.
api/oss/tests/pytest/unit/sessions/test_query_sessions_search.py#L30-L58: move these four classes into a shared module (for example aconftest.pyfixture or a_dummy_engine.pytest helper underapi/oss/tests/pytest/unit/sessions/) and import them here.api/oss/tests/pytest/unit/sessions/test_query_sessions_windowing.py#L135-L163: import the same shared classes instead of redefining them.web/packages/agenta-entities/src/session/api/api.ts (1)
294-308: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRegenerate the Fern client to include
searchbefore merge.
querySessionsforwards the request object withbody: request, sosearchwill reach the wire in the current SDK; regenerateSessionQueryRequestso the cast grows the generated type instead of relying on this one-off widening.api/oss/src/core/sessions/turns/service.py (1)
112-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
latest_turn_per_sessionreturns a raw dict instead of a DTO.
latest_turn_per_sessionreturnsDict[str, SessionTurn]. The coding guideline requires service methods to return typed DTOs, not raw dicts. Every other method in this service (query_turns,latest_turn,latest_turn_per_harness_kind) followsList[DTO]/Optional[DTO]. This method is the exception.A full fix wraps the mapping in a small DTO (for example
LatestTurnsBySessionwith aturns: Dict[str, SessionTurn]field). This also touchesSessionTurnsDAOInterface.latest_turn_per_session,SessionTurnsDAO.latest_turn_per_session, and the consumer inSessionsService.query_sessions, plus the test fixtures that assert on the bare dict today.Given the wide blast radius versus the marginal type-safety gain over an already-typed
Dict[str, SessionTurn], treat this as deferable.As per coding guidelines: "Service methods must return typed DTOs (Pydantic
BaseModelsubclasses), not raw dicts, tuples, orAny; useOptional[DTO]for missing entities andList[DTO]for collections."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93a2da23-f32a-427c-8806-56d84f4b6b15
📒 Files selected for processing (18)
api/oss/src/apis/fastapi/sessions/models.pyapi/oss/src/apis/fastapi/sessions/router.pyapi/oss/src/core/sessions/dtos.pyapi/oss/src/core/sessions/service.pyapi/oss/src/core/sessions/streams/dtos.pyapi/oss/src/core/sessions/turns/interfaces.pyapi/oss/src/core/sessions/turns/service.pyapi/oss/src/dbs/postgres/sessions/streams/dao.pyapi/oss/src/dbs/postgres/sessions/turns/dao.pyapi/oss/src/dbs/postgres/shared/utils.pyapi/oss/tests/pytest/unit/sessions/test_query_sessions_references.pyapi/oss/tests/pytest/unit/sessions/test_query_sessions_search.pyapi/oss/tests/pytest/unit/sessions/test_query_sessions_windowing.pyapi/oss/tests/pytest/unit/sessions/test_sessions_root_service.pyweb/oss/src/components/AgentChatSlice/state/projectSessions.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/tests/unit/session-query-schema.test.ts
| /** | ||
| * Pins the `/sessions/query` wire shape for `sessionStreamSchema`/`sessionsQueryResponseSchema`. | ||
| * | ||
| * Fern's compile-time types under-declare backend `extra="allow"` fields and don't catch | ||
| * server-side field renames, and zod silently STRIPS unknown wire keys to `undefined` on a | ||
| * `.nullish()` field — so a renamed backend key (e.g. `name`, `references`) tsc-passes and | ||
| * parse-succeeds while the FE session list silently loses the data (this class of drift has | ||
| * bitten the session schemas twice: see `session-record-schema.test.ts`). These tests assert | ||
| * a realistic wire payload survives parsing with its values intact, and document — via a | ||
| * deliberately-renamed fixture — that a real rename would slip past zod undetected unless | ||
| * this fixture is kept in sync with an actual backend payload. | ||
| */ | ||
| import {describe, expect, it} from "vitest" | ||
|
|
||
| import {sessionsQueryResponseSchema, sessionStreamSchema} from "../../src/session/core/schema" | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Prettier formatting failure reported by CI.
The pipeline log reports a Prettier formatting failure for cd web && pnpm run format in this change. Run pnpm lint-fix (or prettier --write) from web on this file and commit the result.
As per coding guidelines: "Run pnpm lint-fix from the web directory before committing."
🧰 Tools
🪛 GitHub Actions: 11 - check code styling / 3_TypeScript format.txt
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues. Command 'cd web && pnpm run format' failed with exit code 1.
🪛 GitHub Actions: 11 - check code styling / TypeScript format
[error] 1-1: Prettier formatting check failed during 'cd web && pnpm run format'. Run Prettier with --write to fix code style issues.
Sources: Coding guidelines, Pipeline failures
4c9c4be to
48a843f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
a8ad264 to
885738f
Compare
48a843f to
a024cad
Compare
|
Prettier on |
Adds an optional case-insensitive substring `search` filter over session_streams.name, threaded from the request model through core SessionQuery/SessionStreamQuery down to the DAO (escaped ilike). FE querySessions gains a matching search param with an interim Fern-type cast until the client is regenerated.
a024cad to
75be3f7
Compare
1 participant
Context
The session list had no useful order.
/sessions/queryreturned rows by creation date, so a session you worked in five minutes ago sat below one you created last week and never touched. There was also no way to find a session by name, and a row carried no hint of what it was about.This is lane 2 of a 12-PR stack (see #5679 for the shape). It is backend work that stands on its own: the desktop list benefits from it immediately, and the mobile app in later lanes depends on it.
Changes
Ordering moves to last activity. The window is
coalesce(updated_at, created_at)rather thanupdated_atalone, because a session that has never been updated has a null there and would sort as if it were the oldest thing in the project. The id tiebreak follows the sort direction, so a page boundary that lands mid-tie does not repeat or skip a row./sessions/querygains a free-text search over the session title, and rows now carry their latest turn's references so a list can label a row without a second round trip per session.The desktop's client-side sort in
AgentChatSlice/state/projectSessions.tsis deleted. The server is now authoritative for order, and keeping a client sort on top of it means two implementations that can disagree.Tests / notes
@agenta/entitiesgains a wire-shape test pinned to a server-faithful fixture, so a backend change that alters the row shape fails on the frontend side too.