Repository navigation
[3370 by Claude Opus 5.5] 0.9.0 release change: make the dashboard AI activity summary (the bar at the top of the dashboard showing Live/Paused/Saved/Syncing/Idle with text such as 'Gathering the latest activity…' or 'Activity overview is temporarily unavailable.',... - #3371
integry merged 5 commits into
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…othing new. Lint, typecheck, release verify and the 42 Playwright tests were already confirmed passing in the summary above, and nothing has changed since.
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR implements the default-off setting and groups the controls appropriately, but needs changes to the dashboard’s enablement gate before merge. ✅ Explicit opt-in — Production configuration reads enable summaries only for a saved boolean ✅ Isolated settings writes — Unrelated settings saves no longer implicitly persist ✅ Experimental grouping — Voice and dashboard controls share an Experimental section, with scope and LLM-cost explanations. The supplied objective is truncated; this review evaluates its visible requirements and the changed behavior. Current checks report no failures, with nine pending. No commands or tests were run. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Cached opt-in bypasses current enablement
F2: 🔴 Status failures permanently suppress recovery
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe configuration and settings changes are well scoped, but two enablement lifecycle defects remain. Added tests cover initial states without covering stale confirmation or recovery; current checks have no reported failures. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
/fix |
|
✅ Applied the requested follow-up changes in commit fa692c8
Summary of ChangesI fixed F1 and F2 in F1: a cached opt-in no longer mounts the summary. Every dashboard mount now asks the server for the setting. The bar and its narrative request only mount once that request says F2: a failed status check can now recover. The status now has four states: pending, on, off and unavailable. Only "unavailable" (the request failed) is checked again, and nothing shows until a check succeeds. A re-check happens when either of these occurs:
A confirmed Tests (
Verification, all passing:
Nothing else in the UI, end-to-end tests or docs read the removed session key. I didn't capture previews: the bar looks the same, and the change only affects when it mounts. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…hboardSummary.tsx` and kept it simple, as you asked: the session-storage opt-in cache is gone entirely. I fixed F1 and F2 in `propr-ui/src/components/Dashboard/DashboardSummary.tsx` and kept it simple, as you asked: the session-storage opt-in cache is gone entirely. **F1: a cached opt-in no longer mounts the summary.** Every dashboard mount now asks the server for the setting. The bar and its narrative request only mount once that request says `on`. If you turn summaries off in Settings and come back, there is no bar and no narrative request. Prose saved in the narrative hook's own cache is still kept, but it only shows after a fresh `on`. **F2: a failed status check can now recover.** The status now has four states: pending, on, off and unavailable. Only "unavailable" (the request failed) is checked again, and nothing shows until a check succeeds. A re-check happens when either of these occurs: - `activityToken` changes. - the page becomes visible again. A confirmed `off` is not re-checked on activity, so a disabled summary makes no extra requests. The listener that watches for the page becoming visible is only attached while the status is unavailable. **Tests** (`DashboardSummary.test.tsx`): - **New tests:** - After an earlier confirmed session, turning the setting off means a remount renders nothing and requests no narrative, both before and after the status reply. - Saved prose only appears after a fresh confirmation. - An unreadable status is re-checked on an activity change and again when the page becomes visible; it stays hidden until it returns `on`, and is not re-checked after that. - A confirmed `off` is not re-checked. - **Changed test:** "restores cached prose" now waits one tick for the status reply before checking for the prose. - **Negative check:** with the previous version of `DashboardSummary.tsx` swapped back in, the first three new tests fail. **Verification, all passing:** - the Dashboard vitest files (108 tests). - `eslint --max-warnings 0` on the Dashboard folder. - `tsc --noEmit`. - the `dashboard-responsive` and `usage-tips` Playwright specs (27 tests). Nothing else in the UI, end-to-end tests or docs read the removed session key. I didn't capture previews: the bar looks the same, and the change only affects when it mounts. PR: #3371 Comment by: @integry (ID: 6491003993) Model: Claude Opus 5.5
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR makes dashboard AI summaries opt-in and groups them with experimental settings. It needs a localized recovery fix before merge. ✅ Explicit opt-in — Production configuration reads require ✅ Fresh confirmation — The dashboard waits for a fresh status response before mounting the narrative consumer, including when cached prose exists. ✅ Focused coverage — Added tests cover hidden defaults, fresh confirmation, and recovery from rejected status requests. The supplied original objective is truncated; this review assesses its visible requirements and the supplied diff. Current checks show 37 passed, none failed, and one pending. No commands or tests were run. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F3: 🔴 Preserve unavailable status for recovery
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Align the remaining settings documentationThe Settings section of ScoreThe implementation covers the primary opt-in behavior well, but the new status endpoint defeats recovery from backend configuration failures. The correction is localized; current checks otherwise provide strong supporting evidence. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
/fix F3 S1 |
|
✅ Applied the requested follow-up changes in commit 99f876d
Summary of ChangesF3 and S1 are both fixed. When the summary setting can't be read, the dashboard now treats it as unavailable and checks again, so an enabled summary no longer stays hidden until the page is remounted. F3: unreadable setting
S1: settings docs ( Checks: the backend status tests pass (2/2), the There's nothing visible to capture, so I made no preview images. Found separately, not changed: the main summary endpoint ( 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
… read, the dashboard now treats it as unavailable and checks again, so an enabled summary no longer stays hidden until the page is remounted.
F3 and S1 are both fixed. When the summary setting can't be read, the dashboard now treats it as unavailable and checks again, so an enabled summary no longer stays hidden until the page is remounted.
**F3: unreadable setting**
- **Backend** (`packages/api/routes/dashboardRoutes.ts`): `getNarrativeStatus` now returns `503 { error: 'Dashboard summary status is unavailable' }` instead of a successful `{ enabled: false }`. The frontend reads that error as `'unavailable'`, so the bar stays hidden and the existing re-checks (on new dashboard activity or when the page becomes visible again) keep running.
- **Backend test** (`packages/api/test/dashboardNarrativeStatus.test.ts`): it used to expect `enabled: false` here; it now expects the 503 and its body.
- **Backend-to-frontend test** (new file, `propr-ui/src/components/Dashboard/DashboardSummary.statusRecovery.test.tsx`): unlike the existing UI tests, it uses the real dashboard API client and only stubs the network. The status request first gets the same 503 the backend sends: nothing renders and no summary is requested. Once the setting becomes readable, the next activity change re-checks, gets `{ enabled: true }`, and the bar appears with one summary request. The UI suite can't call the backend handler directly, so the two tests are tied together by using the same 503 body.
- **Negative check:** I swapped the stub back to the old `200 { enabled: false }` response and the new test failed (the bar never appeared), so it does catch this bug.
- **Other callers:** `DashboardSummary` is the only caller of the status endpoint, so nothing else had the same problem.
**S1: settings docs** (`docs/docs/features/web-ui.md`): the Integrations bullet no longer lists experimental voice controls. A new **General** bullet says Experimental is where voice briefings and dashboard AI summaries are turned on. I also added **General** to the tab list in the line above, which otherwise wouldn't match the new bullet.
**Checks:** the backend status tests pass (2/2), the `DashboardSummary` UI tests pass (22/22), and ESLint with zero warnings plus `tsc --noEmit` pass for both `packages/api` and `propr-ui`. I didn't run the full suites.
There's nothing visible to capture, so I made no preview images.
**Found separately, not changed:** the main summary endpoint (`getNarrative`) has a similar fallback. When the setting can't be read, it returns `enabled: true, summary: null`. That was already there before this PR and wasn't part of F3, so I left it alone. I also didn't touch the OpenAPI entry for the status endpoint. It still says "not documented yet" and doesn't list the 503, though its generic error response covers it.
PR: #3371
Comment by: @integry (ID: 6491801694)
Model: Claude Opus 5.5
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationNeeds a localized correction before merge. The opt-in defaults and Settings organization are implemented well, but production configuration failures still bypass the new recovery behavior. ✅ Explicit opt-in persistence — Unrelated settings saves omit ✅ Fresh confirmation gate — The dashboard requests no narrative and renders no bar until status confirms enablement, with tests covering unavailable-status recovery. The supplied original issue description is truncated, so requirements beyond its visible portion could not be assessed. The supplied diff was reviewed as complete. No commands or tests were run. Current checks report no failures, with 22 pending. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F4: 🔴 Production status reads suppress recoverable failures
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe implementation substantially satisfies the visible objective and adds useful coverage, but the production failure path still prevents recovery after a transient configuration outage. Pending checks alone do not lower this assessment. Score: 6/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
/fix F4 |
|
✅ Applied the requested follow-up changes in commit 9b028ca
Summary of ChangesI fixed F4. The production Changes
Verification
Sibling check: There's no visible UI change, so I didn't create a preview. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…uses core's existing strict reader, `getConfigStrict`. A missing `dashboard_summary_enabled` setting still means off. A failed read or an unparseable value now makes the callback throw, so `getNarrativeStatus` returns its retryable 503 instead of a 200 with `{ enabled: false }`.
I fixed F4. The production `isSummaryEnabled` callback now uses core's existing strict reader, `getConfigStrict`. A missing `dashboard_summary_enabled` setting still means off. A failed read or an unparseable value now makes the callback throw, so `getNarrativeStatus` returns its retryable 503 instead of a 200 with `{ enabled: false }`.
**Changes**
- **`packages/core/src/config/configManager.ts`:** `getConfigStrict` is now exported from `@propr/core` alongside `getConfig`. It already existed in `configStore.ts`, but the package didn't expose it.
- **`packages/api/routes/dashboardSummarySetting.ts`** (new, a few lines): `isDashboardSummaryEnabled()` returns `(await getConfigStrict('dashboard_summary_enabled', false)) === true`. I moved it out of the inline lambda so tests can exercise the production callback.
- **`packages/api/server.ts`:** passes `isSummaryEnabled: isDashboardSummaryEnabled` and drops the now-unused `getConfig` import.
- **`packages/api/test/dashboardNarrativeStatus.test.ts`:** a new regression test wires the real callback into `getNarrativeStatus` over a migrated SQLite database. It checks four states:
- no setting → 200 `{ enabled: false }`
- `true` → 200 `{ enabled: true }`
- a value that isn't valid JSON → 503
- the config table renamed away, simulating a failed read → 503, then 200 `{ enabled: true }` once it's restored
- **`DashboardSummary.statusRecovery.test.tsx`:** comment only. It now points to the backend test that produces the 503 body the frontend recovery test consumes, linking the two halves of the sequence in F4.
**Verification**
- The new backend test passes (3/3 in that file).
- Negative check: with `getConfigStrict` swapped back to `getConfig`, the new test fails, so it catches the original defect.
- The frontend status-recovery test passes.
- `tsc --noEmit` for `packages/api` passes after rebuilding core.
- `eslint --max-warnings 0` passes on the changed files in `packages/api` and `packages/core`.
**Sibling check:** `getNarrative` calls the same `deps.isSummaryEnabled`, so it now gets a rejection rather than a silent false. That lands in its existing `try`. The test suites that mock `@propr/core` copy the whole real module and only import the MCP server, so the new export doesn't affect them.
There's no visible UI change, so I didn't create a preview.
PR: #3371
Comment by: @integry (ID: 6491989832)
Model: Claude Opus 5.5
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR makes dashboard AI summaries opt-in, groups them under Experimental settings, and preserves the existing API and summarization backend. It is merge-ready within the supplied scope, conditional on pending checks completing successfully. ✅ Explicit opt-in — Production enables summaries only for a saved boolean ✅ Safe recovery — The strict configuration reader propagates read and parse failures to a 503 response. The frontend remains hidden and retries unavailable status on activity or restored visibility. ✅ Fresh confirmation — The narrative consumer mounts only after status confirmation, with regression coverage for cached prose and backend failure recovery. This was a static review; no commands or tests were run. The supplied issue objective is truncated, so requirements beyond its visible portion could not be assessed. Current checks report 29 passed, seven pending, and no failures. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S2: 🟢 Align the route fallback
ScoreThe visible implementation satisfies the opt-in behavior and includes focused regression coverage for confirmation and recovery. No merge-blocking defect was verified; pending checks still need to finish. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
|
CI failed: Validate Changes Please investigate and fix this CI failure.
Failure evidence
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 360ada7 Processed 2 comments:
Summary of ChangesI fixed the Validate Changes failure. It was the test that was out of date, not the app. Cause: This PR moved the administrator's voice briefings opt-in into the new shared Experimental section under General. The non-admin account view still uses its own "Voice briefings · Experimental" heading. The smoke test Fix: In Verification: I ran 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…was out of date, not the app. I fixed the Validate Changes failure. It was the test that was out of date, not the app. **Cause:** This PR moved the administrator's voice briefings opt-in into the new shared **Experimental** section under General. The non-admin account view still uses its own "Voice briefings · Experimental" heading. The smoke test `shows the administrator opt-in under General` still looked for that old heading on the admin Settings page, so it couldn't find it. **Fix:** In `propr-ui/e2e/mobile-voice-briefing.smoke.pw.ts:301`, the test now checks for the `Experimental` heading, matched exactly. The rest of the test is unchanged: the "Enable voice briefings" checkbox starts unchecked, the Voice briefing button is hidden, and checking the box shows the button. **Verification:** I ran `mobile-voice-briefing.smoke.pw.ts` in Playwright and both of its tests pass. ESLint with `--max-warnings 0` is clean on that file. I didn't make previews because only a test changed, so there's nothing new to see. PR: #3371 Comment by: @propr-dev[bot] (ID: 6493197693) Comment by: @github-actions[bot] (ID: 6493198428) Model: Claude Opus 5.5
AI Implementation Summary
Closes #3370
Branch:
3370/claude-opus-5-5-0-9-0-release-change-make-20261029-0715-387Commits: ✅ Changes committed (9d1348e)
AI Processing Completed
Execution Details:
Summary:
That last notification was a wait task I'd started earlier; it adds nothing new. Lint, typecheck, release verify and the 42 Playwright tests were already confirmed passing in the summary above, and nothing has changed since.
Detailed Logs:
cc80b850-4389-45d9-aca6-b0e8f128f39fLog files stored at:
/usr/src/app/logs/executions/issue-3370-2026-10-29T07-35-21-018Z-conversation.json/usr/src/app/logs/executions/issue-3370-2026-10-29T07-35-21-018Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3370.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!
Visual preview
After: dashboard with the setting unset
No summary bar and no narrative request; the panes start directly under the toolbar.
Original unavailable: managed storage is unavailable or the upload has expired.
After: Settings → General → Experimental
One Experimental section containing Enable voice briefings and Dashboard AI summaries (unchecked by default), noting the summarization model and LLM cost.
Original unavailable: managed storage is unavailable or the upload has expired.
After: AI Models & Reasoning → Planning
The checkbox is removed from Model selection; the Summarization Model helper points to Experimental.
Original unavailable: managed storage is unavailable or the upload has expired.
After: settings search for 'dashboard summary'
Searching 'dashboard summary' finds the Experimental section; 'experimental' and 'voice' find it too.
Original unavailable: managed storage is unavailable or the upload has expired.
Before: dashboard with default settings
The AI activity summary bar showed above the panes by default.
Original unavailable: managed storage is unavailable or the upload has expired.
Before: Settings → General
Voice briefings had its own 'Voice briefings · Experimental' section; the dashboard summary toggle lived elsewhere.
Original unavailable: managed storage is unavailable or the upload has expired.
Before: AI Models & Reasoning → Planning
The Dashboard summary checkbox sat under Model selection, checked by default.
Original unavailable: managed storage is unavailable or the upload has expired.