feat(weekly-brief): AI Assistant card on committee overview#778
Draft
manishdixitlfx wants to merge 9 commits into
Draft
feat(weekly-brief): AI Assistant card on committee overview#778manishdixitlfx wants to merge 9 commits into
manishdixitlfx wants to merge 9 commits into
Conversation
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR titles must follow Conventional Commits. Love from, Your reviewers ❤️.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR wires up the WG Weekly Brief “AI Assistant” card end-to-end on the Committee Overview page, adds shared throttle defaults, and introduces Playwright e2e coverage for key card states and transitions.
Changes:
- Added a new
WeeklyBriefCardComponent(template + logic) and mounted it above “My Pending Actions” on committee overview behind an OpenFeature flag. - Centralized default throttle counters into a shared constant consumed by both the BFF and Angular fallback paths.
- Added Playwright e2e tests for feature-flag gating, empty/generated states, generate-from-empty, and edit→save round-trip.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/constants/weekly-brief.constants.ts | Adds shared default weekly-brief throttle constants. |
| packages/shared/src/constants/index.ts | Re-exports weekly-brief constants from the shared constants barrel. |
| apps/lfx-one/src/server/services/weekly-brief.service.ts | Uses shared throttle defaults and adds COMMITTEE_SERVICE_URL override routing for live calls. |
| apps/lfx-one/src/server/controllers/weekly-brief.controller.ts | Updates route comments to reflect /api/... paths. |
| apps/lfx-one/src/app/shared/services/weekly-brief.service.ts | Uses shared throttle defaults for empty-envelope fallback and updates logging. |
| apps/lfx-one/src/app/shared/components/textarea/textarea.component.ts | Adds dataTestId input for data-testid support. |
| apps/lfx-one/src/app/shared/components/textarea/textarea.component.html | Binds data-testid attribute from the new dataTestId input. |
| apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/weekly-brief-card.component.ts | Introduces the AI Assistant card component behavior (fetch, generate, edit/save, copy). |
| apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/weekly-brief-card.component.html | Implements card UI for loading/empty/generated/edit states and throttling display. |
| apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/weekly-brief-card.component.scss | Adds stylesheet placeholder (license header). |
| apps/lfx-one/src/app/modules/committees/components/committee-overview/committee-overview.component.ts | Adds feature-flag signal and imports the weekly brief card component. |
| apps/lfx-one/src/app/modules/committees/components/committee-overview/committee-overview.component.html | Conditionally renders the weekly brief card based on flag + edit permissions. |
| apps/lfx-one/e2e/weekly-brief-card.spec.ts | Adds Playwright e2e coverage for weekly brief card gating and key flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
force-pushed
the
feat/wg-weekly-brief-angular-service
branch
from
8952067 to
6baa70b
Compare
…w integration Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
- Reorder class members per component-organization.md (injections before inputs; refresh$ in helpers band) - Replace raw textarea + (input) handler with ReactiveForms FormControl - Approved badge: green -> emerald (brand success scale) - Add data-testid attributes on 8 interactive elements + empty state - Guard onGenerate against re-entrant clicks while generating - Log clipboard write failure via console.warn for DataDog RUM Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Covers: feature-flag gating, empty state, generated state, edit/save round-trip, and generate-from-empty happy path. Closes learnings-reviewer finding on missing empty-state e2e coverage. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Position the AI Assistant card between the meetings section and pending actions for better discoverability. Visible to all users with writer access. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Allow COMMITTEE_SERVICE_URL env var to bypass LFX_V2_SERVICE gateway when testing against a locally-running committee-service instance. Falls back to LFX_V2_SERVICE proxy when not set. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…vice override Address post-commit code-reviewer findings on 4b4069cf: - Use req.bearerToken (matches MicroserviceProxyService) instead of the raw Authorization header, so OIDC / impersonation / refreshed tokens flow correctly into the local-dev override path. - Throw MicroserviceError.fromMicroserviceResponse so upstream 429/409 status codes propagate through apiErrorHandler instead of collapsing to a generic 500. - Guard empty / 204 No Content responses before JSON.parse to avoid SyntaxError on success paths. - Widen method union to match proxyRequest's accepted methods. - Add logger.debug to document the dispatch decision. - Document that the direct-fetch bypass is deliberate for the local-dev override (vs MicroserviceProxyService) so reviewers don't re-flag it. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Consolidated post-commit + full-branch sweep findings: - Throw MicroserviceError on 204/empty body instead of returning undefined-cast-as-T (fixes generic-return-type-lies Critical). - Tighten getCurrentBrief catch to instanceof MicroserviceError; drop dead error?.status === 404 defensiveness. - Add /api prefix to weekly-brief controller JSDoc (matches mounted path in server.ts). - Promote throttle defaults to WEEKLY_BRIEF_DEFAULT_THROTTLE shared constant; consume from BFF and Angular client so future policy changes propagate (fixes hardcoded-list-duplicates-config). - Guard onSave against empty/whitespace brief text. - Pipe onGenerate/onSave subscriptions through take(1) per frontend checklist §6 (no bare .subscribe). - Replace raw <textarea> with lfx-textarea wrapper per checklist §2. - Log via console.warn when client catchError swallows an HTTP error so real upstream failures aren't silenced (combined with the BFF's 404-to-empty-envelope normalization). Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
added a commit
that referenced
this pull request
May 24, 2026
Address copilot-pull-request-reviewer comments:
- apps/lfx-one/src/server/services/weekly-brief.service.ts
(proxyToCommitteeService):
* URL: use `new URL(path, overrideUrl)` so trailing-slash variants on
COMMITTEE_SERVICE_URL don't produce `//path` or drop the path.
* Defaults: append DEFAULT_QUERY_PARAMS (`v=1`) on the override path so
behaviour matches MicroserviceProxyService.proxyRequest.
* Timeout: 15s AbortSignal.timeout(...) — a hung local committee-
service no longer wedges the BFF request.
* Body parse: read response.text() first, then JSON.parse with a
catch — empty/non-JSON 2xx bodies now surface as 502 BAD_GATEWAY
via MicroserviceError instead of a raw SyntaxError 500.
* 204 / empty body: map to 502 (the weekly-brief endpoints always
return a payload on success) instead of leaking the raw 204 status
through MicroserviceError.
- apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/
weekly-brief-card.component.ts (weekLabel + onGenerate):
* weekLabel: format window_start / window_end with `timeZone: 'UTC'`
so users in negative offsets don't see the start shift to the
prior day (window_start/end are documented as UTC Sun→Sat
boundaries).
* onGenerate: explicit branch for 409 (edited-brief guard) — prompt
reload and refresh$.next() instead of swallowing as "Failed to
generate brief". Distinct from 429 (throttle exhausted).
- apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/
weekly-brief-card.component.html:
* Generating branch now fires on `generating() || brief()?.state ===
'generating'` — backend-side persisted-generating state no longer
falls through to the normal brief UI.
* Added explicit `brief()?.state === 'error'` branch with a Try
again button (uses canGenerate gating).
- apps/lfx-one/e2e/weekly-brief-card.spec.ts (mockCurrentBrief):
* Helper is now async and awaits page.route() — no more race with
page.goto() that could let real-network requests leak through.
* All four call sites updated to await the helper.
Resolves 14 review threads (the 3 doc-related threads — about
wg-weekly-brief-rebuild-prompts.md / -implementation-status.md — are
addressed by the rebase that dropped those commits; the files no
longer exist in this branch).
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
force-pushed
the
feat/wg-weekly-brief-ui-card
branch
from
3a094fc to
6504343
Compare
Contributor
Author
Review Feedback AddressedCommit: 6504343 Changes Made (per copilot-pull-request-reviewer)
Scope cleanupThe two planning docs ( Threads Resolved14 of 14 unresolved threads addressed in this iteration. |
Address copilot-pull-request-reviewer comments:
- apps/lfx-one/src/server/services/weekly-brief.service.ts
(proxyToCommitteeService):
* URL: use `new URL(path, overrideUrl)` so trailing-slash variants on
COMMITTEE_SERVICE_URL don't produce `//path` or drop the path.
* Defaults: append DEFAULT_QUERY_PARAMS (`v=1`) on the override path so
behaviour matches MicroserviceProxyService.proxyRequest.
* Timeout: 15s AbortSignal.timeout(...) — a hung local committee-
service no longer wedges the BFF request.
* Body parse: read response.text() first, then JSON.parse with a
catch — empty/non-JSON 2xx bodies now surface as 502 BAD_GATEWAY
via MicroserviceError instead of a raw SyntaxError 500.
* 204 / empty body: map to 502 (the weekly-brief endpoints always
return a payload on success) instead of leaking the raw 204 status
through MicroserviceError.
- apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/
weekly-brief-card.component.ts (weekLabel + onGenerate):
* weekLabel: format window_start / window_end with `timeZone: 'UTC'`
so users in negative offsets don't see the start shift to the
prior day (window_start/end are documented as UTC Sun→Sat
boundaries).
* onGenerate: explicit branch for 409 (edited-brief guard) — prompt
reload and refresh$.next() instead of swallowing as "Failed to
generate brief". Distinct from 429 (throttle exhausted).
- apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/
weekly-brief-card.component.html:
* Generating branch now fires on `generating() || brief()?.state ===
'generating'` — backend-side persisted-generating state no longer
falls through to the normal brief UI.
* Added explicit `brief()?.state === 'error'` branch with a Try
again button (uses canGenerate gating).
- apps/lfx-one/e2e/weekly-brief-card.spec.ts (mockCurrentBrief):
* Helper is now async and awaits page.route() — no more race with
page.goto() that could let real-network requests leak through.
* All four call sites updated to await the helper.
Resolves 14 review threads (the 3 doc-related threads — about
wg-weekly-brief-rebuild-prompts.md / -implementation-status.md — are
addressed by the rebase that dropped those commits; the files no
longer exist in this branch).
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Refs: LFXV2-1984
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
manishdixitlfx
force-pushed
the
feat/wg-weekly-brief-angular-service
branch
from
6baa70b to
192bb9b
Compare
manishdixitlfx
force-pushed
the
feat/wg-weekly-brief-ui-card
branch
from
6504343 to
d18610e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracking: LFXV2-1984 (part of Epic LFXV2-1976)
Summary
Wires up the WG Weekly Brief end-to-end: AI Assistant card component on the Committee Overview page, with Playwright e2e coverage. Largest of the 8 PRs (component + template + styles + tests + several review-iteration fixes folded in).
What's new
apps/lfx-one/src/app/modules/committees/components/weekly-brief-card/— component (TS + HTML + SCSS) covering all card states: empty, generating, generated, edited, approved, error, throttle-exceeded, edit-modeapps/lfx-one/src/app/modules/committees/components/committee-overview/— moves the AI Assistant card above pending actionsapps/lfx-one/src/app/shared/components/textarea/— small enhancement for the edit-mode textareapackages/shared/src/constants/weekly-brief.constants.ts—WEEKLY_BRIEF_DEFAULT_THROTTLEapps/lfx-one/e2e/weekly-brief-card.spec.ts— Playwright spec covering card state transitionsCOMMITTEE_SERVICE_URLenv override on the BFF: lets you point at a locally-running committee-service without retargetingLFX_V2_SERVICEglobally (dev convenience)Notes
refresh$ = new BehaviorSubject<void>(undefined)is a refresh trigger (legitimate RxJS pattern, not simple state)signal()(fetchLoading, generating, saving, editMode)fix(review): ...commits in this branch addressed reviewer findings on prior iterations of this card — they're folded in here for a clean diffStack
PR 4/4 on lfx-self-serve. Base = PR #777 (angular-service).
Test plan
yarn lint/yarn buildgreen (verified)yarn e2e -- weekly-brief-card.spec.tspasses