|
1 | | -# Android Question Chat polish pass v2 report |
| 1 | +# Android Question Chat polish pass v3 report |
2 | 2 |
|
3 | 3 | ## Scope completed |
4 | 4 |
|
5 | | -Completed on the current branch, test-first: |
6 | | -- Removed the visible `Question Chat` header/title and the ready/state pill. |
7 | | -- Replaced chat status chrome with a single accessibility-described workspace border: |
8 | | - - green when connected/usable |
9 | | - - red when offline/stale/unavailable/not yet usable |
10 | | -- Removed the large outer white chat card and used the screen background directly. |
11 | | -- Replaced the text `Send` action with a circular send bubble icon while preserving accessible `Send` / `Steer` labels. |
12 | | -- Kept `Stop` available in generating/stopping states. |
13 | | -- Made the bottom `Question` / `Chat` nav always visible for pending questions before activation. |
14 | | -- Renamed the second tab to `Chat` and removed the dedicated in-question Chat button. |
15 | | -- Wired preactivation Chat tab taps to explicit exact-fork activation, with loading/error/context-fallback states rendered inside the Chat tab. |
16 | | -- Styled `Add a note` to match the web spotlight action more closely. |
17 | | -- Switched the activity to `adjustResize` and removed the extra composer IME padding so the composer stays anchored above the resized bottom area instead of double-padding upward. |
18 | | -- Updated JVM + Compose/instrumentation seams for: |
19 | | - - workspace/tab behavior before activation |
20 | | - - explicit activation selection |
21 | | - - border status behavior |
22 | | - - send/steer accessibility labels |
23 | | - - removal of legacy header/pill/button chrome |
24 | | - - preserved callbacks and bottom placement |
25 | | - |
26 | | -## Source inspection performed before editing |
27 | | - |
28 | | -Inspected the relevant native/web/source evidence first: |
29 | | -- `CONTEXT.md` |
30 | | -- `IMPLEMENTATION_REPORT.md` |
31 | | -- `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionChatUi.kt` |
32 | | -- `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionWorkflowScreen.kt` |
33 | | -- `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionWorkflowViewModel.kt` |
34 | | -- `apps/android/app/src/main/java/dev/pi/postbox/questionchat/QuestionChatWorkspaceShell.kt` |
35 | | -- `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionChatUiTest.kt` |
36 | | -- `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionWorkflowScreenTest.kt` |
37 | | -- `apps/android/app/src/test/java/dev/pi/postbox/question/QuestionWorkflowQuestionChatTest.kt` |
38 | | -- `apps/android/app/src/test/java/dev/pi/postbox/questionchat/QuestionChatWorkspaceShellTest.kt` |
39 | | -- `apps/web/src/components/QuestionLayoutSpotlight.svelte` |
40 | | -- `apps/web/src/components/QuestionDetail.svelte` |
41 | | -- `apps/web/src/components/QuestionChatActivation.svelte` |
42 | | -- `apps/web/src/styles.css` |
43 | | -- existing screenshot evidence under `/tmp/android-chat-ui-evidence/` |
| 5 | +Completed on the current branch after inspecting the interrupted diff first and keeping only the useful production/test work: |
| 6 | +- While generating, the composer action now swaps in-place from Send/Steer to a Stop icon button in the same circular slot, with accessible label `Stop` and no separate Stop button. |
| 7 | +- The chat transcript is now a bounded weighted `LazyColumn` history above a bottom composer, with one production IME inset ownership path on the composer. |
| 8 | +- Starter answers are now compact visual pills with preserved button semantics and 48dp minimum interactive height. |
| 9 | +- The Question action row now uses web-matching copy and hierarchy: `Submit` + icon and `Add note` + icon on one responsive row, with `Cancel` below. |
| 10 | +- Added focused Compose/instrumentation coverage for: |
| 11 | + - send/stop action swapping in one slot |
| 12 | + - compact starter semantics vs visual size |
| 13 | + - one-row Submit/Add note copy and layout |
| 14 | + - bounded real-IME probe/geometry seam with bounded skip instead of hanging the emulator |
| 15 | +- Preserved the useful screenshot/evidence harness, removed the unused `TestComposeImeActivity` / androidTest manifest experiment, and kept the minimal debug host activity actually used by the committed instrumentation. |
| 16 | + |
| 17 | +## Library rationale |
| 18 | + |
| 19 | +Researched maintained chat UI kits before editing. |
| 20 | + |
| 21 | +Not adopted: |
| 22 | +- GetStream `stream-chat-android-ai-compose` / `ChatComposer` |
| 23 | + |
| 24 | +Reason: |
| 25 | +- it is maintained and does cover keyboard/composer/chat concerns, |
| 26 | +- but it pulls in Stream-specific licensing/ecosystem assumptions plus attachment/voice/message abstractions and a visual/state model this app would mostly override, |
| 27 | +- while backend-neutral maintained Compose chat kits remain sparse for this narrow app-owned Question Chat. |
| 28 | + |
| 29 | +Decision: |
| 30 | +- keep the app-owned UI, |
| 31 | +- use the standard bounded `LazyColumn` + anchored composer pattern from current Compose inset guidance, |
| 32 | +- preserve the existing deep owner / safe renderer architecture. |
| 33 | + |
| 34 | +Compose insets reference consulted before editing: |
| 35 | +- fetched Compose inset-consumption guidance under `/tmp/rpiv-fetch-S93HR6/content.txt` (nested `imePadding` / inset consumption behavior) |
| 36 | +- accepted workspace contract in `docs/prototypes/2026-07-29-android-question-chat-workspace.md` |
| 37 | +- current interrupted task brief in `/tmp/android-chat-polish3.md` |
| 38 | +- rereview findings in `/tmp/android-chat-t1-rereview-report.md` |
| 39 | + |
| 40 | +## Diff inspection performed first |
| 41 | + |
| 42 | +Inspected the full interrupted working tree before changing anything: |
| 43 | +- `git status --short` |
| 44 | +- `git diff --stat` |
| 45 | +- full diffs for all modified tracked files |
| 46 | +- contents of the new androidTest/debug harness files |
| 47 | +- current production/test files and the web action row source |
44 | 48 |
|
45 | 49 | ## Changed files |
46 | 50 |
|
47 | | -- `apps/android/app/src/main/AndroidManifest.xml` |
| 51 | +Production: |
48 | 52 | - `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionChatUi.kt` |
49 | 53 | - `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionWorkflowScreen.kt` |
50 | | -- `apps/android/app/src/main/java/dev/pi/postbox/question/QuestionWorkflowViewModel.kt` |
51 | | -- `apps/android/app/src/main/java/dev/pi/postbox/questionchat/QuestionChatWorkspaceShell.kt` |
52 | 54 | - `apps/android/app/src/main/java/dev/pi/postbox/ui/theme/PostalDecorations.kt` |
| 55 | + |
| 56 | +Compose/instrumentation tests: |
53 | 57 | - `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionChatUiTest.kt` |
54 | 58 | - `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionWorkflowScreenTest.kt` |
55 | | -- `apps/android/app/src/test/java/dev/pi/postbox/question/QuestionWorkflowQuestionChatTest.kt` |
56 | | -- `apps/android/app/src/test/java/dev/pi/postbox/questionchat/QuestionChatWorkspaceShellTest.kt` |
| 59 | +- `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionChatImeGeometryTest.kt` |
| 60 | +- `apps/android/app/src/androidTest/java/dev/pi/postbox/question/QuestionChatEvidenceCaptureTest.kt` |
| 61 | +- `apps/android/app/src/debug/AndroidManifest.xml` |
| 62 | +- `apps/android/app/src/debug/java/dev/pi/postbox/question/QuestionTestHostActivity.kt` |
| 63 | + |
| 64 | +Docs: |
57 | 65 | - `IMPLEMENTATION_REPORT.md` |
58 | 66 |
|
59 | | -Note: |
60 | | -- Temporary screenshot-only instrumentation helpers were created, used, and removed before final validation. |
61 | | -- A temporary detached-HEAD worktree was used to capture the `before-*` evidence from the pre-change code. |
| 67 | +## Evidence |
62 | 68 |
|
63 | | -## Screenshot evidence |
| 69 | +Evidence directory: |
| 70 | +- `/tmp/android-chat-ui-evidence-v3/` |
64 | 71 |
|
65 | | -Captured readable before/after screenshots under: |
66 | | -- `/tmp/android-chat-ui-evidence-v2/` |
| 72 | +Captured after screenshots: |
| 73 | +- `after-idle-composer.png` |
| 74 | +- `after-generating-stop-composer.png` |
| 75 | +- `after-starter-pills.png` |
| 76 | +- `after-question-action-row.png` |
67 | 77 |
|
68 | | -Files: |
69 | | -- `before-ready.png` |
70 | | -- `after-ready.png` |
71 | | -- `before-disconnected.png` |
72 | | -- `after-disconnected.png` |
73 | | -- `before-composer-ime.png` |
74 | | -- `after-composer-ime.png` |
75 | | -- `before-preactivation-question-bottom-nav.png` |
76 | | -- `after-preactivation-question-bottom-nav.png` |
| 78 | +Retained before screenshots: |
| 79 | +- `before-idle-composer.png` |
| 80 | +- `before-generating-stop-composer.png` |
| 81 | +- `before-starter-pills.png` |
| 82 | +- `before-question-action-row.png` |
| 83 | +- `before-keyboard-visible.png` |
77 | 84 |
|
78 | | -IME note: |
79 | | -- The composer focus/inset evidence was captured on `emulator-5554` after tapping the composer on API 26 with `show_ime_with_hard_keyboard=1` and resize mode enabled. |
80 | | -- On this emulator build, the reserved IME space was reflected in the app layout during capture, but the software keyboard surface itself was not included in the screenshot output. |
| 85 | +Important IME note: |
| 86 | +- The committed `QuestionChatImeGeometryTest` performs a bounded real-IME probe on `emulator-5554` and skips instead of hanging when this API-26 headless emulator never surfaces a software keyboard. |
| 87 | +- I did **not** fabricate an `after-keyboard-visible.png` screenshot on this runner. |
| 88 | +- See `/tmp/android-chat-ui-evidence-v3/NOTES.md`. |
81 | 89 |
|
82 | 90 | ## Commands run |
83 | 91 |
|
84 | | -- environment / repo inspection: |
85 | | - - `env | grep '^PI_' | sort` |
86 | | - - `git status --short --branch` |
87 | | - - `ls`, `find`, `grep`, `read` |
88 | | -- focused red phase: |
89 | | - - `cd apps/android && ./gradlew testDebugUnitTest connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class='dev.pi.postbox.question.QuestionChatUiTest,dev.pi.postbox.question.QuestionWorkflowScreenTest'` |
90 | | -- focused JVM validation: |
91 | | - - `cd apps/android && ./gradlew testDebugUnitTest --tests 'dev.pi.postbox.question.QuestionWorkflowQuestionChatTest' --tests 'dev.pi.postbox.questionchat.QuestionChatWorkspaceShellTest'` |
92 | | -- focused connected validation: |
93 | | - - `cd apps/android && ./gradlew connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class='dev.pi.postbox.question.QuestionChatUiTest,dev.pi.postbox.question.QuestionWorkflowScreenTest'` |
94 | | -- before evidence capture from temporary worktree at pre-change `HEAD`: |
95 | | - - `git worktree add /tmp/pi-postbox-android-chat-before HEAD` |
96 | | - - `adb -s emulator-5554 shell settings put secure show_ime_with_hard_keyboard 1` |
97 | | - - `cd /tmp/pi-postbox-android-chat-before/apps/android && ./gradlew installDebug installDebugAndroidTest` |
98 | | - - `adb -s emulator-5554 shell am instrument -w -e class dev.pi.postbox.question.QuestionChatEvidenceCaptureTest -e prefix before dev.pi.postbox.test/androidx.test.runner.AndroidJUnitRunner` |
99 | | - - `adb -s emulator-5554 exec-out run-as dev.pi.postbox cat /data/user/0/dev.pi.postbox/files/android-chat-ui-evidence-v2/<before-file> > /tmp/android-chat-ui-evidence-v2/<before-file>` |
100 | | - - `git worktree remove /tmp/pi-postbox-android-chat-before --force` |
101 | | -- after evidence capture on current code: |
102 | | - - `cd apps/android && ./gradlew installDebug installDebugAndroidTest` |
103 | | - - `adb -s emulator-5554 shell am instrument -w -e class dev.pi.postbox.question.QuestionChatEvidenceCaptureTest -e prefix after dev.pi.postbox.test/androidx.test.runner.AndroidJUnitRunner` |
104 | | - - `adb -s emulator-5554 exec-out run-as dev.pi.postbox cat /data/user/0/dev.pi.postbox/files/android-chat-ui-evidence-v2/<after-file> > /tmp/android-chat-ui-evidence-v2/<after-file>` |
105 | | -- final full Android gates: |
106 | | - - `cd apps/android && ./gradlew testDebugUnitTest connectedDebugAndroidTest lintDebug assembleDebug assembleDebugAndroidTest` |
107 | | -- diff hygiene: |
108 | | - - `git diff --check` |
| 92 | +Environment / repo inspection: |
| 93 | +- `env | sort | grep '^PI_'` |
| 94 | +- `git status --short` |
| 95 | +- `git diff --stat` |
| 96 | +- full `git diff ...` over the interrupted file set |
| 97 | +- `read`, `grep`, `find`, `ls` |
109 | 98 |
|
110 | | -## Validation status |
| 99 | +Focused verification: |
| 100 | +- `cd apps/android && ./gradlew testDebugUnitTest --tests 'dev.pi.postbox.question.QuestionWorkflowQuestionChatTest' --tests 'dev.pi.postbox.questionchat.QuestionChatWorkspaceShellTest'` |
| 101 | +- `cd apps/android && ./gradlew connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class='dev.pi.postbox.question.QuestionChatUiTest,dev.pi.postbox.question.QuestionWorkflowScreenTest,dev.pi.postbox.question.QuestionChatImeGeometryTest'` |
111 | 102 |
|
112 | | -Passed: |
113 | | -- Focused JVM tests for `QuestionWorkflowQuestionChatTest` and `QuestionChatWorkspaceShellTest` |
114 | | -- Focused connected Compose/instrumentation tests for `QuestionChatUiTest` and `QuestionWorkflowScreenTest` on `emulator-5554` |
115 | | -- Full Android gates: |
116 | | - - `cd apps/android && ./gradlew testDebugUnitTest connectedDebugAndroidTest lintDebug assembleDebug assembleDebugAndroidTest` |
| 103 | +Evidence capture: |
| 104 | +- `cd apps/android && ./gradlew installDebug installDebugAndroidTest` |
| 105 | +- `adb -s emulator-5554 shell am instrument -w -e class dev.pi.postbox.question.QuestionChatEvidenceCaptureTest -e prefix after dev.pi.postbox.test/androidx.test.runner.AndroidJUnitRunner` |
| 106 | +- `adb -s emulator-5554 exec-out run-as dev.pi.postbox cat /data/user/0/dev.pi.postbox/files/android-chat-ui-evidence-v3/<file> > /tmp/android-chat-ui-evidence-v3/<file>` |
| 107 | + |
| 108 | +Emulator recovery / bounded retry work: |
| 109 | +- restarted the API-26 emulator with bounded waits after System UI ANR loops during IME experiments: |
| 110 | + - `/home/dev/Android/Sdk/emulator/emulator @postbox_api26_tcg -no-window -no-snapshot-load -no-snapshot-save -gpu swiftshader_indirect -accel off` |
| 111 | + |
| 112 | +Final Android gates: |
| 113 | +- `cd apps/android && ./gradlew testDebugUnitTest connectedDebugAndroidTest lintDebug assembleDebug assembleDebugAndroidTest` |
117 | 114 |
|
118 | | -## Residual risks / notes |
| 115 | +Diff hygiene: |
| 116 | +- `git diff --check` |
119 | 117 |
|
120 | | -- `LocalClipboardManager` deprecation warnings remain outside this UI slice. |
121 | | -- The old in-question activation helper functions were removed from the active path; Chat activation now happens through the persistent bottom nav / workspace shell path. |
| 118 | +## Validation status |
| 119 | + |
| 120 | +Passed: |
| 121 | +- focused JVM tests for `QuestionWorkflowQuestionChatTest` and `QuestionChatWorkspaceShellTest` |
| 122 | +- focused connected tests for `QuestionChatUiTest` and `QuestionWorkflowScreenTest` |
| 123 | +- full Android gates: |
| 124 | + - `testDebugUnitTest` |
| 125 | + - `connectedDebugAndroidTest` |
| 126 | + - `lintDebug` |
| 127 | + - `assembleDebug` |
| 128 | + - `assembleDebugAndroidTest` |
| 129 | + |
| 130 | +Skipped by design after bounded probing: |
| 131 | +- `QuestionChatImeGeometryTest` |
| 132 | +- `QuestionChatEvidenceCaptureTest` cases in the default full suite (they require explicit evidence prefix) |
| 133 | + |
| 134 | +## Residual notes |
| 135 | + |
| 136 | +- The API-26 headless emulator on this runner never surfaced a real IME under bounded automation, even after switching soft keyboards and restarting the emulator without snapshots. The committed IME probe now skips rather than hanging the suite or fabricating proof. |
| 137 | +- `LocalClipboardManager` deprecation warnings remain outside this polish slice. |
122 | 138 | - No push performed. |
123 | 139 | - No PR opened. |
0 commit comments