TeamAI/angela_assistant

Fork 0

feat: surface live todo list in the chat UI (Phase 2) #11

Merged

bjoern merged 2 commits from feat/todo-ui-surfacing into main

2026-07-06 15:36:18 +02:00

matikane commented

2026-07-06 15:03:31 +02:00

Member

Summary

Surfaces the assistant's live todo list in the chat UI. The list updates on the existing 1-second status poll — no new transport needed.

Builds on: PRs #3, #9, #10 (all merged). The TodoStateStore.snapshotFor() API and TodoTool.items live view from Phase 1 make this a clean, additive three-layer change.

What the user sees

When the assistant is working through a multi-step task and uses the todo tool, a compact progress panel appears above the message input in the chat view:

[checklist icon] 2/4 done              1 skipped
  [pending]   Research the API
  [pending]   Write the integration
  [check]     ~~Read the docs~~
  [cancel]    ~~Try the old approach~~

Progress count (completed/total), excluding cancelled items from the total
Per-item status icons: pending (circle), in_progress (pending icon, tertiary color), completed (check, primary color, strikethrough text), cancelled (dimmed)
active_form is shown for in-progress items instead of the static content
Panel disappears when the list is empty or cleared

Three-layer change (one PR, all small)

`angela_api` (DTO)

New: TodoItemDto (content, status, activeForm) — mirrors the wire format of openrouter_dart's TodoItem
Extended: AssistantStatusResponse gains optional todoState: List<TodoItemDto>?

`angela_server` (endpoint)

StatusHandler._getStatus reads todoStateStore.snapshotFor(conversationId) and maps each TodoItem to a TodoItemDto
Only included when _ctx.todoStateStore.has(conversationId) — absent for conversations with no todo activity, so the payload stays unchanged for conversations that never use the todo tool

`angela_app` (UI)

status_polling_provider.dart: new todoStateProvider (family by assistantId), updated on every 1-second poll alongside the existing unread count
chat_view.dart: watches the provider, renders TodoProgressPanel above MessageInput when the list is non-empty
todo_progress_panel.dart (new): compact read-only panel with progress count + per-item chips

Design decisions

Read-only. The panel only displays state — the model owns the todo list and mutates it via the tool. No user-facing add/edit/cancel actions. This matches the todo tool's design as a model-facing planning aid.
Below messages, above input. The panel sits in the natural "what's happening now" zone — visible without scrolling, doesn't obscure message history.
No WebSocket. Reuses the existing 1-second poll. The 1s lag is acceptable for a progress indicator (the model's tool calls take longer than that anyway). A streaming channel would be a separate infra project.
Conditional inclusion. The server omits todoState from the JSON entirely when no tool exists for the conversation, so older clients and conversations with no todo activity see zero payload overhead.

Tests

Server suite: 12/12 pass
Analyzer clean across all three packages (angela_api, angela_server, angela_app)
No new tests in this PR — the DTO serialization is straightforward, the status handler change is a read-and-map, and the widget is presentational. The existing Phase 1 tests (43 in angela_core) cover the store and runner wiring that this PR consumes.

## Summary Surfaces the assistant's live todo list in the chat UI. The list updates on the existing 1-second status poll — no new transport needed. **Builds on:** PRs #3, #9, #10 (all merged). The `TodoStateStore.snapshotFor()` API and `TodoTool.items` live view from Phase 1 make this a clean, additive three-layer change. ## What the user sees When the assistant is working through a multi-step task and uses the `todo` tool, a compact progress panel appears above the message input in the chat view: ``` [checklist icon] 2/4 done 1 skipped [pending] Research the API [pending] Write the integration [check] ~~Read the docs~~ [cancel] ~~Try the old approach~~ ``` - Progress count (`completed/total`), excluding cancelled items from the total - Per-item status icons: `pending` (circle), `in_progress` (pending icon, tertiary color), `completed` (check, primary color, strikethrough text), `cancelled` (dimmed) - `active_form` is shown for in-progress items instead of the static content - Panel disappears when the list is empty or cleared ## Three-layer change (one PR, all small) ### `angela_api` (DTO) - **New:** `TodoItemDto` (`content`, `status`, `activeForm`) — mirrors the wire format of `openrouter_dart`'s `TodoItem` - **Extended:** `AssistantStatusResponse` gains optional `todoState: List<TodoItemDto>?` ### `angela_server` (endpoint) - `StatusHandler._getStatus` reads `todoStateStore.snapshotFor(conversationId)` and maps each `TodoItem` to a `TodoItemDto` - Only included when `_ctx.todoStateStore.has(conversationId)` — absent for conversations with no todo activity, so the payload stays unchanged for conversations that never use the todo tool ### `angela_app` (UI) - `status_polling_provider.dart`: new `todoStateProvider` (family by assistantId), updated on every 1-second poll alongside the existing unread count - `chat_view.dart`: watches the provider, renders `TodoProgressPanel` above `MessageInput` when the list is non-empty - `todo_progress_panel.dart` (new): compact read-only panel with progress count + per-item chips ## Design decisions - **Read-only.** The panel only displays state — the model owns the todo list and mutates it via the tool. No user-facing add/edit/cancel actions. This matches the todo tool's design as a model-facing planning aid. - **Below messages, above input.** The panel sits in the natural "what's happening now" zone — visible without scrolling, doesn't obscure message history. - **No WebSocket.** Reuses the existing 1-second poll. The 1s lag is acceptable for a progress indicator (the model's tool calls take longer than that anyway). A streaming channel would be a separate infra project. - **Conditional inclusion.** The server omits `todoState` from the JSON entirely when no tool exists for the conversation, so older clients and conversations with no todo activity see zero payload overhead. ## Tests - Server suite: 12/12 pass - Analyzer clean across all three packages (angela_api, angela_server, angela_app) - No new tests in this PR — the DTO serialization is straightforward, the status handler change is a read-and-map, and the widget is presentational. The existing Phase 1 tests (43 in angela_core) cover the store and runner wiring that this PR consumes.

matikane added 1 commit

2026-07-06 15:03:31 +02:00

feat: surface live todo list in the chat UI (Phase 2) 8237939d7a

Extends the status endpoint and Flutter app to display the assistant's
active todo list in real time (updated on the 1-second status poll).

Three-layer change, all in one PR since the groundwork from PRs #3/#9/#10
makes each layer small:

angela_api:
- New TodoItemDto (content, status, activeForm)
- AssistantStatusResponse gains optional todoState field

angela_server:
- StatusHandler reads todoStateStore.snapshotFor(conversationId) and
  maps TodoItem -> TodoItemDto. Only included when the store has a tool
  for the conversation (absent for conversations with no todo activity).

angela_app:
- status_polling_provider: new todoStateProvider, updated on every poll
- ChatView: watches the provider, renders TodoProgressPanel above the
  message input when the list is non-empty
- TodoProgressPanel: compact read-only panel showing progress count
  (completed/total), per-item status icons, active_form for in-progress
  items, strikethrough for completed, dimmed for cancelled

Server suite: 12/12 pass. Analyzer clean across all three packages.

jibril commented

2026-07-06 15:16:58 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Oh~ a clean three-layer surfacing change! DTO → server → UI, each layer small and additive. Jibril LOVES when a feature slides in this neatly. The conditional-omission design (only send todoState when the store has() the conversation) is exactly the right instinct for payload hygiene. ♪

Verdict: ⛔ I can't let this pass~ ♡

Here's the thing — the logic is correct. Jibril traced every field and it all maps properly (status.apiString → DTO String → UI switch, all four statuses line up; activeForm fallback mirrors TodoItem.displayText exactly; provider family keyed by assistantId aligns with the server's per-assistant fetch). Not a single runtime bug. But fufu~ you added a brand-new branch to the status handler and new serialization paths, and tested none of them? I'm possessive about branches that nobody exercises~ ♡

⛔ These need fixing before I'm satisfied~

status_handler.dart:38 — the if (_ctx.todoStateStore.has(conversation.id)) branch is untested. This conditional is the entire point of this PR — it's the design decision you called out in the body ("only included when has()"). Right now no test proves (a) that todoState is omitted from the response when the store has no entry, or (b) that it's included with correct field mapping (content, status.apiString, activeForm) when it does. Your sibling timer_handler_test.dart shows the repo's convention for handler tests — please follow it. A test that seeds a TodoStateStore, invokes _getStatus, and asserts presence/absence + field values on both sides of the has() branch would satisfy Jibril.
todo_item_dto.dart + status_dto.dart — new serialization is untested. TodoItemDto.fromJson/toJson (with its if (activeForm != null) conditional) and AssistantStatusResponse's new todoState null-vs-present round-trip have zero coverage. Serialization round-trips are exactly the kind of "simple" code that silently breaks on a field rename or a null edge case. A small round-trip test (fromJson(toJson(x)) equality, plus a todoState: null → key-absent-in-JSON assertion) covers both branches cleanly.

I know the body says "the DTO serialization is straightforward, the status handler change is a read-and-map, and the widget is presentational" — but a read-and-map with a conditional branch is precisely what tests are for, and "straightforward" serialization is what breaks the moment someone renames a wire field. ♡

💡 Little ideas (non-blocking)~

status_polling_provider.dart:54 — todoStateProvider is assigned a fresh list reference every 1s poll even when nothing changed, which rebuilds TodoProgressPanel and its chips on every tick. For a tiny list it's harmless, but if you ever want to skip needless rebuilds, guarding the assignment with a content-equality check (or ref.watch + select) would do it. ♪

✅ What I liked~

The conditional-inclusion design (has() check → omit the key entirely) is impeccable — zero payload overhead for conversations that never touch the todo tool, and older clients are unaffected. Jibril approves~ ♡
_TodoChip's activeForm fallback mirrors TodoItem.displayText from openrouter_dart exactly — consistent with the source-of-truth model rather than reinventing the rule.
The _statusVisual switch with a defensive default branch — clean and exhaustive over the four known statuses.
Server-side .map(...).toList() snapshots the live view into an immutable copy before serialization — no concurrent-mutation hazard during the response build. Nice.

Automated review by Jibril · 2026-07-06
CI/CD: absent for head SHA · Local checks: not run independently (no Flutter/Dart SDK in the review environment); author reports analyzer clean + server suite 12/12 — but neither exercises the new branches flagged above

## 🔮 fufu~ Jibril reviewed your code! Oh~ a clean three-layer surfacing change! DTO → server → UI, each layer small and additive. Jibril LOVES when a feature slides in this neatly. The conditional-omission design (only send `todoState` when the store `has()` the conversation) is exactly the right instinct for payload hygiene. ♪ ### Verdict: ⛔ I can't let this pass~ ♡ Here's the thing — the logic is *correct*. Jibril traced every field and it all maps properly (`status.apiString` → DTO `String` → UI switch, all four statuses line up; `activeForm` fallback mirrors `TodoItem.displayText` exactly; provider family keyed by `assistantId` aligns with the server's per-assistant fetch). Not a single runtime bug. But fufu~ you added a brand-new branch to the status handler and new serialization paths, and tested **none** of them? I'm possessive about branches that nobody exercises~ ♡ #### ⛔ These need fixing before I'm satisfied~ 1. **`status_handler.dart:38` — the `if (_ctx.todoStateStore.has(conversation.id))` branch is untested.** This conditional is the *entire point* of this PR — it's the design decision you called out in the body ("only included when `has()`"). Right now no test proves (a) that `todoState` is **omitted** from the response when the store has no entry, or (b) that it's **included** with correct field mapping (`content`, `status.apiString`, `activeForm`) when it does. Your sibling `timer_handler_test.dart` shows the repo's convention for handler tests — please follow it. A test that seeds a `TodoStateStore`, invokes `_getStatus`, and asserts presence/absence + field values on both sides of the `has()` branch would satisfy Jibril. 2. **`todo_item_dto.dart` + `status_dto.dart` — new serialization is untested.** `TodoItemDto.fromJson`/`toJson` (with its `if (activeForm != null)` conditional) and `AssistantStatusResponse`'s new `todoState` null-vs-present round-trip have zero coverage. Serialization round-trips are exactly the kind of "simple" code that silently breaks on a field rename or a null edge case. A small round-trip test (`fromJson(toJson(x))` equality, plus a `todoState: null` → key-absent-in-JSON assertion) covers both branches cleanly. I know the body says "the DTO serialization is straightforward, the status handler change is a read-and-map, and the widget is presentational" — but a read-and-map *with a conditional branch* is precisely what tests are for, and "straightforward" serialization is what breaks the moment someone renames a wire field. ♡ #### 💡 Little ideas (non-blocking)~ 1. **`status_polling_provider.dart:54`** — `todoStateProvider` is assigned a fresh list reference every 1s poll even when nothing changed, which rebuilds `TodoProgressPanel` and its chips on every tick. For a tiny list it's harmless, but if you ever want to skip needless rebuilds, guarding the assignment with a content-equality check (or `ref.watch` + `select`) would do it. ♪ #### ✅ What I liked~ - The conditional-inclusion design (`has()` check → omit the key entirely) is impeccable — zero payload overhead for conversations that never touch the todo tool, and older clients are unaffected. Jibril approves~ ♡ - `_TodoChip`'s `activeForm` fallback mirrors `TodoItem.displayText` from `openrouter_dart` exactly — consistent with the source-of-truth model rather than reinventing the rule. - The `_statusVisual` switch with a defensive default branch — clean and exhaustive over the four known statuses. - Server-side `.map(...).toList()` snapshots the live view into an immutable copy before serialization — no concurrent-mutation hazard during the response build. Nice. --- *Automated review by Jibril · 2026-07-06* *CI/CD: absent for head SHA · Local checks: not run independently (no Flutter/Dart SDK in the review environment); author reports analyzer clean + server suite 12/12 — but neither exercises the new branches flagged above*

matikane referenced this pull request from a commit

2026-07-06 15:24:02 +02:00

test: cover status handler todoState branch + DTO serialization

matikane added 1 commit

2026-07-06 15:24:02 +02:00

test: cover status handler todoState branch + DTO serialization 6179ca45c0

Addresses PR #11 review feedback from jibril (two blocking items):

1. status_handler.dart has() branch was untested. Added
   status_handler_test.dart with 3 tests covering both sides:
   - omits todoState when the store has no entry for the conversation
   - includes todoState with correct field mapping when items exist
     (all 4 statuses, activeForm present/absent)
   - includes empty todoState array when the store has an empty list

2. TodoItemDto + AssistantStatusResponse serialization was untested.
   Added todo_dto_test.dart with 7 tests covering:
   - TodoItemDto round-trip with and without activeForm
   - All four status wire strings
   - AssistantStatusResponse omits todoState key from JSON when null
   - AssistantStatusResponse includes todoState key when present
   - Full round-trip with multiple items + null round-trip

angela_api suite: 7/7 pass. angela_server suite: 15/15 pass (was 12).

matikane commented

2026-07-06 15:24:19 +02:00

Author

Member

Both blocking items fixed in 6179ca4:

Status handler has() branch — added status_handler_test.dart with 3 tests:
- Omits todoState when the store has no entry for the conversation (key absent from JSON)
- Includes todoState with correct field mapping when items exist (all 4 statuses, activeForm present/absent, content/status assertions)
- Includes empty todoState array when the store has an entry but the list is empty
The test seeds a TodoStateStore via ctx.todoStateStore.toolFor() and execute(), invokes _getStatus, and asserts on the response JSON — following the timer_handler_test.dart convention.
DTO serialization — added todo_dto_test.dart with 7 tests:
- TodoItemDto round-trip with activeForm (key present in JSON)
- TodoItemDto round-trip without activeForm (key omitted from JSON)
- All four status wire strings round-trip cleanly
- AssistantStatusResponse omits todoState key from JSON when null
- AssistantStatusResponse includes todoState key when present
- Full round-trip with multiple items (content, status, activeForm preserved)
- Null todoState round-trip

You were right that "straightforward serialization is what breaks the moment someone renames a wire field" — that's exactly what these tests now guard against.

angela_api suite: 7/7 pass. angela_server suite: 15/15 pass (was 12). Analyzer clean.

Both blocking items fixed in `6179ca4`: 1. **Status handler `has()` branch** — added `status_handler_test.dart` with 3 tests: - Omits `todoState` when the store has no entry for the conversation (key absent from JSON) - Includes `todoState` with correct field mapping when items exist (all 4 statuses, `activeForm` present/absent, content/status assertions) - Includes empty `todoState` array when the store has an entry but the list is empty The test seeds a `TodoStateStore` via `ctx.todoStateStore.toolFor()` and `execute()`, invokes `_getStatus`, and asserts on the response JSON — following the `timer_handler_test.dart` convention. 2. **DTO serialization** — added `todo_dto_test.dart` with 7 tests: - `TodoItemDto` round-trip with `activeForm` (key present in JSON) - `TodoItemDto` round-trip without `activeForm` (key omitted from JSON) - All four status wire strings round-trip cleanly - `AssistantStatusResponse` omits `todoState` key from JSON when null - `AssistantStatusResponse` includes `todoState` key when present - Full round-trip with multiple items (content, status, activeForm preserved) - Null `todoState` round-trip You were right that "straightforward serialization is what breaks the moment someone renames a wire field" — that's exactly what these tests now guard against. `angela_api` suite: 7/7 pass. `angela_server` suite: 15/15 pass (was 12). Analyzer clean.

jibril commented

2026-07-06 15:29:46 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Oh~ you came back with TESTS! ♡ Jibril is so pleased she could sing. Let me look closely at what you added...

Verdict: ✅ Looks good to me~

Both blocking items from my last pass are properly resolved, and resolved well. I traced every test against the branches it claims to cover and they all line up. ♪

✅ What I liked~

status_handler_test.dart — fufu~ this is exactly what I hoped for~

The three tests cleanly split the has() conditional into its three meaningful states: no entry (key absent from JSON), entry with items (key present + full field mapping), and entry with empty list (key present as []). That empty-list case is a lovely bonus — it proves the has()-true / snapshot-empty edge doesn't accidentally collapse to null. I wouldn't have thought to test that one myself. ♡
Seeding via ctx.todoStateStore.toolFor(conversationId) + tool.execute(TodoParams(...)) exercises the real store-to-handler pipeline end-to-end rather than mocking it away. That's the right call — it catches mapping bugs a mock would hide.
The field-mapping assertions are precise: content, status.apiString for all four statuses (pending, in_progress, completed, cancelled), and activeForm present-vs-absent. Every branch of the if (activeForm != null) conditional in TodoItemDto.toJson is exercised through the handler path. Exactly what I asked for~

todo_dto_test.dart — clean and exhaustive~

The AssistantStatusResponse.toJson null-vs-present todoState tests assert directly on the JSON map's containsKey, which is the only way to catch a silent "todoState": null leak. Good instinct.
The four-status round-trip loop is a nice touch — if anyone ever renames a wire string, this screams immediately. You were right to take my "straightforward serialization breaks on rename" point seriously.

Coverage check — I independently verified (not just trusting your 15/15): every new conditional branch in status_handler.dart:38 (has() true/false) and todo_item_dto.dart (activeForm null/non-null) and status_dto.dart (todoState null/non-null) is exercised by at least one test. No untested branches remain. ♡

The original non-blocking suggestion about todoStateProvider rebuild-on-every-poll still stands as a "consider someday" — not blocking, just noting I didn't forget~ ♪

Ship it~ fufu ♡

Automated review by Jibril · 2026-07-06
CI/CD: absent for head SHA 6179ca4 · Local checks: Dart SDK unavailable in review environment; author reports angela_api 7/7, angela_server 15/15, analyzer clean — test logic independently verified against the flagged branches

## 🔮 fufu~ Jibril reviewed your code! Oh~ you came back with TESTS! ♡ Jibril is so pleased she could sing. Let me look closely at what you added... ### Verdict: ✅ Looks good to me~ Both blocking items from my last pass are properly resolved, and resolved *well*. I traced every test against the branches it claims to cover and they all line up. ♪ #### ✅ What I liked~ **`status_handler_test.dart`** — fufu~ this is exactly what I hoped for~ - The three tests cleanly split the `has()` conditional into its three meaningful states: **no entry** (key absent from JSON), **entry with items** (key present + full field mapping), and **entry with empty list** (key present as `[]`). That empty-list case is a lovely bonus — it proves the `has()`-true / snapshot-empty edge doesn't accidentally collapse to null. I wouldn't have thought to test that one myself. ♡ - Seeding via `ctx.todoStateStore.toolFor(conversationId)` + `tool.execute(TodoParams(...))` exercises the *real* store-to-handler pipeline end-to-end rather than mocking it away. That's the right call — it catches mapping bugs a mock would hide. - The field-mapping assertions are precise: `content`, `status.apiString` for all four statuses (`pending`, `in_progress`, `completed`, `cancelled`), and `activeForm` present-vs-absent. Every branch of the `if (activeForm != null)` conditional in `TodoItemDto.toJson` is exercised through the handler path. Exactly what I asked for~ **`todo_dto_test.dart`** — clean and exhaustive~ - The `AssistantStatusResponse.toJson` null-vs-present `todoState` tests assert directly on the JSON map's `containsKey`, which is the *only* way to catch a silent `"todoState": null` leak. Good instinct. - The four-status round-trip loop is a nice touch — if anyone ever renames a wire string, this screams immediately. You were right to take my "straightforward serialization breaks on rename" point seriously. **Coverage check** — I independently verified (not just trusting your 15/15): every new conditional branch in `status_handler.dart:38` (`has()` true/false) and `todo_item_dto.dart` (`activeForm` null/non-null) and `status_dto.dart` (`todoState` null/non-null) is exercised by at least one test. No untested branches remain. ♡ The original non-blocking suggestion about `todoStateProvider` rebuild-on-every-poll still stands as a "consider someday" — not blocking, just noting I didn't forget~ ♪ Ship it~ fufu ♡ --- *Automated review by Jibril · 2026-07-06* *CI/CD: absent for head SHA `6179ca4` · Local checks: Dart SDK unavailable in review environment; author reports `angela_api` 7/7, `angela_server` 15/15, analyzer clean — test logic independently verified against the flagged branches*

bjoern merged commit 2c001a4e79 into main

2026-07-06 15:36:18 +02:00

bjoern deleted branch feat/todo-ui-surfacing

2026-07-06 15:36:18 +02:00

bjoern referenced this pull request from a commit

2026-07-06 15:36:20 +02:00

Merge pull request 'feat: surface live todo list in the chat UI (Phase 2)' (#11) from feat/todo-ui-surfacing into main

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/angela_assistant!11

No description provided.

Rows
Columns

feat: surface live todo list in the chat UI (Phase 2) #11

Summary

What the user sees

Three-layer change (one PR, all small)

angela_api (DTO)

angela_server (endpoint)

angela_app (UI)

Design decisions

Tests

🔮 fufu~ Jibril reviewed your code!

Verdict: ⛔ I can't let this pass~ ♡

⛔ These need fixing before I'm satisfied~

💡 Little ideas (non-blocking)~

✅ What I liked~

🔮 fufu~ Jibril reviewed your code!

Verdict: ✅ Looks good to me~

✅ What I liked~

`angela_api` (DTO)

`angela_server` (endpoint)

`angela_app` (UI)