Skip to content

Wire per-row Rename in chat history overflow menu#8599

Merged
GerardPaligot merged 5 commits into
developfrom
feature/gerard/chat-history-rename
May 20, 2026
Merged

Wire per-row Rename in chat history overflow menu#8599
GerardPaligot merged 5 commits into
developfrom
feature/gerard/chat-history-rename

Conversation

@GerardPaligot
Copy link
Copy Markdown
Contributor

@GerardPaligot GerardPaligot commented May 18, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/task/1214820120386822

Description

Wires the Rename action on the Duck.ai chat history screen so it actually edits a conversation's title.

Steps to test this PR

Note

Prerequisites:

  • Install Internal Debug.
  • In Settings → Developer Settings → Feature Flags, confirm duckAiChatHistory (self, historyScreen) is ON and duckChat → useNativeStorageChatData is ON.
  • Create at least 2 chats in Duck.ai so there's something to rename.

Happy path

  • Open the Chats screen → tap the 3-dot on any row → tap Rename → confirm the Rename screen opens with the current title preselected and the keyboard up.
  • Edit the title → confirm the Save (check) action enables only when the field is non-blank AND different from the original.
  • Tap Save → confirm the screen closes and the row in the list now shows the new title without a manual refresh.
  • Open web sidebar and check the chat is renamed there too

Guards

  • Open Rename and clear the field → confirm Save stays disabled.
  • Open Rename and reset the field back to the original title (including the same value with surrounding whitespace) → confirm Save stays disabled.
  • Open Rename and tap the toolbar back arrow / system back → confirm the screen closes and the title is unchanged in the list.

UI changes

Before After
image image

Note

Medium Risk
Adds a new rename flow that updates persisted chat JSON via the native store and introduces new navigation/event plumbing; mistakes could lead to failed updates or incorrect chat metadata updates. Scope is contained to chat history UI and storage layer with good test coverage.

Overview
Enables the Rename action in chat history: selecting rename now emits a NavigationEvent from ChatHistoryViewModel that ChatHistoryFragment handles by pushing a new RenameChatFragment.

Adds a dedicated rename UI (toolbar + single-line title input with Save enabled only when non-blank and changed) backed by RenameChatViewModel, which trims input, calls ChatHistoryRepository.renameChat, and shows an error snackbar on failure.

Plumbs rename through persistence by adding renameChat to ChatHistoryRepository and implementing it in RealDuckAiChatStore to update only the title field in the stored chat JSON; adds strings, menu/layout resources, and unit tests covering navigation events and rename success/failure/malformed JSON cases.

Reviewed by Cursor Bugbot for commit c62b930. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor Author

GerardPaligot commented May 18, 2026

@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch 2 times, most recently from c7dff14 to 2ebe300 Compare May 18, 2026 15:11
@GerardPaligot GerardPaligot marked this pull request as ready for review May 18, 2026 16:42
Comment thread duckchat/duckchat-impl/src/main/res/layout/fragment_rename_chat.xml Outdated
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch from 2ebe300 to b7d6e93 Compare May 19, 2026 08:48
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-native-fire branch from fc2d91c to 23c4ac2 Compare May 19, 2026 08:48
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch 2 times, most recently from 8207298 to cd1d1b5 Compare May 19, 2026 12:38
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit cd1d1b5. Configure here.

Comment thread duckchat/duckchat-impl/src/main/res/layout/fragment_rename_chat.xml
@malmstein malmstein self-assigned this May 19, 2026
Copy link
Copy Markdown
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not install or smoke-test the APK. The rename flow is well-scoped and well-tested across the three layers (store, repository, ViewModel). Two questions worth resolving before merge: rename doesn't bump lastEdit (so the relabelled row stays in place rather than surfacing to the top) and rename has no sync hook while deletes do — likely intentional but worth confirming with design and sync teams. The runCatching in RenameChatViewModel swallows CancellationException and the check { ... } in the repository uses exceptions for control flow; both minor.

Comment thread duckchat/duckchat-impl/src/main/res/menu/menu_rename_chat.xml Outdated
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch from cd1d1b5 to f7b5cd6 Compare May 19, 2026 21:51
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-native-fire branch from 23ad28e to 3f58bc3 Compare May 19, 2026 21:51
Copy link
Copy Markdown
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not install or smoke-test the APK — the debug-apk workflow doesn't run on PRs targeting non-develop branches, and this one is stacked on feature/gerard/chat-history-native-fire. The wiring looks clean — channel-backed navigation events, FragmentScope VM with the standard @ContributesViewModel, ADS components in the layout, @AppCoroutineScope matches the established pattern for saves that should survive widget detach, and tests cover repository, view model and store paths including the malformed-JSON case.

Base automatically changed from feature/gerard/chat-history-native-fire to develop May 20, 2026 08:14
runCatching catches Throwable, so the previous shape silently absorbed
CancellationException and surfaced it as a RenameResult.Error. That
breaks structured concurrency: a cancelled scope should propagate
upwards, not be reported as a rename failure. Swap to a try/catch
that rethrows CancellationException and only converts Exception into
the user-visible error state.
ChatHistoryRepository.renameChat now returns a Boolean: true when the
store applied the rename, false when it could not (chat missing, JSON
malformed). The ViewModel branches on that flag and emits the same
Error outcome for both the false path and any thrown exception — the
fragment shows one generic snackbar either way. Drops the check{}
that turned a soft failure into IllegalStateException and removes the
unused throwable field on RenameResult.Error.
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch from f7b5cd6 to 65337eb Compare May 20, 2026 08:25
The confirm action read its visible label from a string named
..._content_description, which lied about the resource's role. Split
into ..._confirm_title ("Save") for android:title and keep
..._confirm_content_description ("Save chat title") for the new
android:contentDescription binding, so each string is wired to the
attribute its name implies.
@GerardPaligot GerardPaligot force-pushed the feature/gerard/chat-history-rename branch from 65337eb to c62b930 Compare May 20, 2026 14:27
@GerardPaligot GerardPaligot merged commit 5f8b947 into develop May 20, 2026
13 checks passed
@GerardPaligot GerardPaligot deleted the feature/gerard/chat-history-rename branch May 20, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants