Skip to content

feat(web): meeting transcript upload — paste, extract, review, teach (flag-gated, dark)#166

Merged
bejranonda merged 25 commits into
mainfrom
feature/meeting-transcript-upload
Jul 18, 2026
Merged

feat(web): meeting transcript upload — paste, extract, review, teach (flag-gated, dark)#166
bejranonda merged 25 commits into
mainfrom
feature/meeting-transcript-upload

Conversation

@bejranonda

@bejranonda bejranonda commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

Full implementation of the meeting-transcript-upload plan (docs/superpowers/plans/2026-07-14-meeting-transcript-upload.md, all 10 tasks). A signed-in project member can paste a meeting transcript into a new /meetings webapp surface, review LLM-extracted decisions/action-items/open-questions (with assignee dropdowns and decision-supersession suggestions), and confirm each into the Brain via the existing teach path. Stateless extraction (POST /api/meetings/extract, flag-gated MEETING_UPLOAD_ENABLED, default off, rate-limited) returns candidates to browser state; nothing persists until confirmed via POST /api/knowledge. No new database tables, no Prisma migration.

Ships dark — deployed behavior is unchanged until an operator sets MEETING_UPLOAD_ENABLED=true. Enable steps are in docs/KNOWN_ISSUES.md §0o.

  • POST /api/knowledge: action_item type, supersedesKnowledgeId (transactional, project-scoped, membership-validated), tagPrefix filter on GET
  • packages/core/src/meeting-extract.ts: pure prompt-build/parse extraction core + project-wide (deliberately not owner-scoped) decision-supersession search
  • POST /api/meetings/extract: flag-gated, rate-limited extraction endpoint
  • /meetings webapp surface wired into the shell (nav, i18n, flag-gated via /api/me capabilities), paste/review/history UI, flag-gated e2e spec
  • Full doc sweep: README, APPROACH, GUIDELINES, KNOWN_ISSUES, REST_API, MCP_TOOLS, meeting-miner protocol

Post-checkpoint hardening (this session)

A mandatory final whole-branch review (run after all 10 tasks landed, before this merge) caught several things per-task review couldn't see:

  • The branch had never been pushed after the first checkpoint — 11 commits had zero real CI signal. Found and fixed a stale pnpm-lock.yaml and two real vitest/Next.js interop gaps (next-auth's module-scope NextAuth() call, and next/headers's cookies() outside request scope) that were blocking every test in apps/web from even running.
  • The documented operator enable path would have 500'd on first use (model override env vars missing from the deploy compose allowlist) — fixed.
  • A same-project teammate's explicitly-private decision could leak into someone else's supersession suggestions — fixed, scoped locally to the one function, no shared scope-filter code touched.
  • Editing an existing action item's tags bypassed the assignee-membership validation that creating one enforces — fixed with a shared, tested helper.
  • A decision's "rejected alternative" was shown in the review UI but silently never persisted — fixed.

CodeRabbit reviewed the branch three times across this push history (one pass hit CodeRabbit's own rate limit and didn't actually review — verified this rather than trusting a stale green check). 11 further findings verified against current code and fixed, including a stale-response bug in the supersede transaction and a case-canonicalization gap that would have made addressed action items silently never surface to their assignee. Two findings deliberately deferred with reasons recorded in KNOWN_ISSUES.md (a pre-existing shared rate-limiter race, and a repo-wide test-infra convention) rather than scope-crept into this branch.

Test plan

  • ✅ CI green: typecheck/test/build, onboarding e2e, authed e2e, CodeRabbit, GitGuardian — all passing on the final commit
  • git diff main..HEAD -- packages/db/prisma/migrations confirmed empty
  • ✅ Every task independently implemented + reviewed (spec compliance + code quality), several with real fix rounds
  • ✅ Final whole-branch review + CodeRabbit findings independently verified against current source, not taken on faith
  • ⬜ Manual flag-on smoke test on a real deployment (operator, before advertising the feature)

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM

…owledge, plus vitest infra

Plan tasks 1-2. Also adds apps/web's first unit-test infrastructure
(vitest.config.ts + test script) — the plan's test files had nowhere to
run without it; apps/web previously had zero unit coverage, Playwright
e2e only. Scoped narrowly to API route handlers (plain functions over
Web Request/Response, no React/DOM needed).

- POST /api/knowledge: action_item type, supersedesKnowledgeId (now
  transactional with the create — a failure between them previously
  could leave a new row live without retiring the old one), for:<email>
  tags validated against real org membership before persisting.
- supersedeKnowledge (knowledge-stats.ts) hardened with an optional
  projectId constraint — closes a cross-project retirement gap shared
  with the pre-existing MCP brain_teach_knowledge tool (teach.ts updated
  too, same fix, same call).
- GET /api/knowledge: tagPrefix filter, applied to the full candidate
  pool before the response limit (not after — avoids silently dropping
  older matches).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a flag-gated meeting transcript extraction and review workflow, extends the knowledge API for action items, tag filtering, and transactional supersession, and scopes supersession checks by project. It also adds runtime configuration, tests, navigation support, documentation, and end-to-end coverage.

Changes

Meeting transcript workflow

Layer / File(s) Summary
Extraction contracts and supersession search
packages/core/src/meeting-extract.ts, packages/core/src/index.ts, packages/core/src/__tests__/meeting-extract.test.ts
Adds typed transcript extraction, response parsing, LLM dispatch, project-scoped semantic candidate lookup, and tests.
Extraction API and runtime controls
apps/web/app/api/meetings/extract/..., packages/core/src/env.ts, .env.example, deploy/docker-compose.yml
Adds the gated extraction endpoint with membership checks, daily rate limiting, model selection, supersession enrichment, member listing, and structured failures.
Flag-gated meetings surface
apps/web/components/brain/..., apps/web/app/api/me/route.ts, apps/web/lib/brain/...
Adds the meetings route and UI for extraction, review, teaching, supersession confirmation, assignee selection, and history browsing.
Meeting documentation and coverage
README.md, docs/..., apps/web/e2e/meetings.spec.ts, .github/workflows/authed-e2e.yml
Documents the workflow and launch controls and adds authenticated browser coverage.

Knowledge API behavior

Layer / File(s) Summary
Knowledge API validation and persistence
apps/web/app/api/knowledge/..., apps/web/lib/brain/assignee-validation.ts, apps/web/vitest.*, apps/web/package.json
Adds action items, assignee validation, tag-prefix filtering, transactional supersession, field preservation, and Vitest route coverage.
Project-scoped supersession propagation
packages/core/src/knowledge-stats.ts, packages/core/src/__tests__/knowledge-supersede.test.ts, apps/mcp-server/src/tools/teach.ts
Scopes supersession lookups by project when context is supplied and forwards resolved project context from MCP teaching.
Knowledge API contract documentation
docs/REST_API.md, docs/MCP_TOOLS.md, docs/GUIDELINES.md
Documents filtering, assignee validation, transactional supersession, and project-scoped ownership rules.

Estimated code review effort: 5 (Critical) | ~100 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant MeetingsUI
  participant ExtractRoute
  participant MeetingExtract
  participant KnowledgeAPI
  User->>MeetingsUI: Paste transcript
  MeetingsUI->>ExtractRoute: POST transcript
  ExtractRoute->>MeetingExtract: Extract decisions and action items
  MeetingExtract-->>ExtractRoute: Return parsed items and candidates
  ExtractRoute-->>MeetingsUI: Return reviewable results and members
  User->>MeetingsUI: Confirm item
  MeetingsUI->>KnowledgeAPI: POST knowledge item
  KnowledgeAPI-->>MeetingsUI: Return created or superseded knowledge
Loading

Estimated code review effort: 5 (Critical) | ~100 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: a dark, flag-gated meeting transcript upload flow with paste, extract, review, and teach steps.
Description check ✅ Passed The description covers the feature, rationale, tests, and rollout, and is mostly complete despite not matching the template headings exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/meeting-transcript-upload

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/api/knowledge/route.test.ts`:
- Around line 11-17: Update the database setup around dbReachable and the guard
for “POST /api/knowledge — action_item + supersedesKnowledgeId + assignee
validation” so an unavailable test database throws or otherwise fails test setup
instead of selecting describe.skip. Require the isolated test database and keep
the tests executing normally when setup succeeds.
- Around line 101-103: Update the test case around ensureDefaultProject so it
creates a separate accessible project rather than reusing the user’s idempotent
default project. Assert the new project ID differs from projectId, then continue
invoking POST without the early return so cross-project supersession is actually
verified.

In `@apps/web/app/api/knowledge/route.ts`:
- Around line 163-184: Update the action_item handling in the knowledge route to
inspect every body.tags entry beginning with "for:", not just the first match.
Lowercase each assignee email, validate every canonicalized assignee against the
project organization members, and persist the tags with each for: value replaced
by its lowercase canonical form so invalid secondary assignees cannot pass and
stored tags match the assignee lookup.
- Around line 90-100: Update the knowledge route’s candidate retrieval and
tagPrefix filtering so matching rows are evaluated across the complete result
set before calculating total and applying limit; avoid relying on the 2,000-row
CANDIDATE_POOL_CAP. In apps/web/app/api/knowledge/route.ts lines 90-100, filter
at the database layer or page through all candidates while preserving ordering
and mapping. In apps/web/app/api/knowledge/route.test.ts lines 187-226, add
coverage with a small limit, newer untagged rows before an older match, and a
match beyond the former candidate boundary.
- Around line 226-233: Update the superseding POST flow around
tx.knowledge.update and the route’s return created path to use the updated row
returned by the update (or reload it before returning), so the 201 payload’s
item.parentKnowledgeId reflects the persisted target.id instead of null. Ensure
the response assertion validates item.parentKnowledgeId as well as the database
row.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 87f468af-e5dc-4d5d-9ff7-8a1779bcd51e

📥 Commits

Reviewing files that changed from the base of the PR and between 3ec49b2 and d636fa8.

📒 Files selected for processing (7)
  • apps/mcp-server/src/tools/teach.ts
  • apps/web/app/api/knowledge/route.test.ts
  • apps/web/app/api/knowledge/route.ts
  • apps/web/package.json
  • apps/web/vitest.config.ts
  • packages/core/src/__tests__/knowledge-supersede.test.ts
  • packages/core/src/knowledge-stats.ts

Comment on lines +11 to +17
const dbReachable = await db
.$queryRaw`SELECT 1`
.then(() => true)
.catch(() => false);
const guard = dbReachable ? describe : describe.skip;

guard("POST /api/knowledge — action_item + supersedesKnowledgeId + assignee validation", () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Do not convert database setup failures into a passing suite.

describe.skip lets CI report success while executing none of these tests. Require an isolated test database and fail setup when it is unavailable.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/knowledge/route.test.ts` around lines 11 - 17, Update the
database setup around dbReachable and the guard for “POST /api/knowledge —
action_item + supersedesKnowledgeId + assignee validation” so an unavailable
test database throws or otherwise fails test setup instead of selecting
describe.skip. Require the isolated test database and keep the tests executing
normally when setup succeeds.

Comment thread apps/web/app/api/knowledge/route.test.ts Outdated
Comment thread apps/web/app/api/knowledge/route.ts Outdated
Comment thread apps/web/app/api/knowledge/route.ts
Comment thread apps/web/app/api/knowledge/route.ts Outdated
bejranonda and others added 13 commits July 17, 2026 14:36
…ist included)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
The brief's test mocks needed updating to match the real LLMDeps interface.
Real signatures: anthropic(prompt, opts), openai(prompt, model, systemPrompt, maxTokens, jsonObject), dashscope(prompt, model, systemPrompt, maxTokens).
Adjusted mocks to accept full parameter lists while only using prompt for recording.
…ot owner-scoped)

findSupersessionCandidates searches for decisions an extracted meeting
decision might replace, scoped to the project — not the caller's own
rows — since decisions are shared team knowledge taught by any member.

Resolves the brief's open question against the real
buildRawProjectFilterV2 contract: its project-scope branch binds a
userId SQL param even with activeProjectId set (legacy
ownerProjectId-IS-NULL fallback, and the private-visibility clause
once accessibleProjectIds is non-empty), so the function takes a real
required userId rather than the brief's empty-string placeholder.

Takes an optional embedFn seam (defaulting to the real embed()),
mirroring extractMeeting's existing LLMDeps injection in this same
file, so the db-guarded test can exercise the real SQL/scope shape
with a fixed vector instead of needing a configured embedding key in
CI.
…action endpoint

Fixes two real bugs in the task brief's draft: rateLimitCheck returns
{ok,...} not {allowed}, and memoryStore is a Store factory, not a Store —
route now goes through getRateLimitStore() like proxy.ts already does.
Also threads the route's resolved userId into findSupersessionCandidates
per its Task-5 contract change (userId is required, not optional).
Adds one test to route.test.ts that mocks meetingExtract.extractMeeting
and findSupersessionCandidates via vi.mock("@brain/core", ...) with a
fixed fake decision/action-item, letting requireOrgMember, rateLimitCheck,
and a real listOrgMembers call run through unmocked. Asserts the exact
final JSON shape (decisions[0].supersedes, actionItems[0], members) so a
broken response-shaping change fails the test, not just a status check.
Review finding from task-6-report.md: the extraction -> supersession ->
member-list -> response chain had never actually been run, only hand-traced.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…flag-gated via /api/me capabilities

Gates both the desktop rail and mobile bottom-nav on the server-reported
meetingUploadEnabled capability (2026-07-10 bug class: no nav entry may
point at an endpoint that 503s). Also fixes Topbar's crumbsMap, which is
an exhaustive Record<Route, ...> that would fail tsc once "meetings"
joined the Route union.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…ask 7 review fix)

Rail and BottomNav are both unconditionally mounted every page load, so
BottomNav's independent useMe() call doubled a genuinely uncached (cache:
"no-store") /api/me network+DB round trip. Follows the same pattern counts
already uses: call useMe() once in BrainApp, pass the result (MeView) as a
NavProps field into both Rail and BottomNav. Also corrects useMe()'s
docstring, which claimed /api/me was cached HTTP-side — contradicted by the
cache: "no-store" fetch option on the next line.
Implements the Meetings client component (Task 8): paste a transcript,
review LLM-extracted decisions/action-items with per-card teach buttons,
and browse prior imports grouped by meeting date. Assignee picker is a
dropdown of real org members only — never free text, and the LLM's
assigneeGuessEmail is pre-selected only when it matches a real member
(server-side re-validated in POST /api/knowledge). Adds the matching
flag-gated e2e spec and wires it into authed-e2e.yml's explicit file
list so it shows as skipped (not silently absent) in CI.
…ponent

Review finding on Task 8 (789f9a7): taught/teaching/cardErrors shared a
single numeric keyspace between decision cards and action-item cards via
a `1000 + index` offset — safe today but not statically prevented if a
transcript ever produced >=1000 decisions. Switch to `d-${index}` /
`a-${index}` string keys so the two lists can never collide by
construction.

decisionSupersedeConfirmed and actionItemAssignee are untouched — they're
already scoped to a single list each with no offset trick.
…CP_TOOLS, KNOWN_ISSUES, GUIDELINES, README, APPROACH)
apps/web/package.json gained vitest ^4.0.0 in Tasks 1-2 (PR #166) but the
lockfile's apps/web importer entry was never updated, so pnpm install
--frozen-lockfile (CI's install step) has been failing with
ERR_PNPM_OUTDATED_LOCKFILE since that commit — meaning no CI run has ever
actually exercised any test in this branch's 12 commits. Regenerated via
`npx pnpm@9.15.0 install --lockfile-only` (pinned to package.json's
declared packageManager version); vitest itself was already fully
resolved elsewhere in the tree, so the diff is the missing importer entry
plus one upstream deprecation-metadata line, not a broader dependency
shift.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…ainer

Final whole-branch review (I1): MEETING_EXTRACT_MODEL/KEA_MODEL/DASHSCOPE_API_KEY
were absent from the web service's compose allowlist, so the qwen3-coder
default (DashScope) would 500 on every extraction on a real deployment
with no way for the operator to override it via .env alone. Added the
passthrough, and fixed the resolution itself (?? -> ||) since compose's
:-} empty-string default would otherwise permanently win over the
KEA_MODEL/default fallback the moment the var was merely allowlisted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 12

🧹 Nitpick comments (1)
apps/web/e2e/meetings.spec.ts (1)

3-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The meeting E2E test is included in CI but never executed.

  • apps/web/e2e/meetings.spec.ts#L3-L11: provide deterministic extraction mocking so the test can run without an LLM provider.
  • .github/workflows/authed-e2e.yml#L179-L179: enable MEETING_UPLOAD_ENABLED for the web process and Playwright command.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/e2e/meetings.spec.ts` around lines 3 - 11, The meeting E2E test is
permanently skipped because its feature flag is disabled in CI and it lacks
deterministic extraction behavior. In apps/web/e2e/meetings.spec.ts lines 3-11,
add deterministic extraction mocking so the meeting surface tests run without an
LLM provider; in .github/workflows/authed-e2e.yml line 179, set
MEETING_UPLOAD_ENABLED=true for both the web process and the Playwright command.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/api/meetings/extract/route.ts`:
- Line 47: Update the request parsing flow around bodySchema.parse in the route
handler to await req.json() separately, catch malformed JSON, and return a 400
invalid_request response through the existing authErrorResponse path. Only pass
successfully parsed JSON to bodySchema.parse, preserving the current validation
handling for schema errors.
- Around line 65-70: The meeting extraction route’s fallback model is not
configurable through Compose and may lack credentials. In
apps/web/app/api/meetings/extract/route.ts lines 65-70, require an explicit
meeting model or retain the DashScope default only when its credentials are
available; in deploy/docker-compose.yml lines 131-132, forward
MEETING_EXTRACT_MODEL and DASHSCOPE_API_KEY when retaining that default. Update
the meetingExtract.extractMeeting flow accordingly without changing the existing
override precedence.
- Around line 72-82: Update the decisionsWithSupersession enrichment around
meetingExtract.findSupersessionCandidates to cap the number of processed
decisions and execute enrichment with bounded concurrency instead of launching
every decision through one unrestricted Promise.all. Preserve the existing
fail-soft catch and supersedes assignment for each processed decision, using the
application’s established cap/concurrency utilities or constants if available.
- Around line 56-63: Replace the non-atomic rateLimitCheck get/set flow used by
the meeting extraction route with an atomic per-key consume operation. Implement
the consume primitive in both Redis and in-memory rate-limit stores, update
rateLimitCheck to use it while preserving the existing limit response behavior,
and add a concurrent-request test proving parallel calls cannot exceed the daily
cap.

In `@apps/web/components/brain/app.tsx`:
- Line 95: Gate the meeting surface with the loaded server capability: in
apps/web/components/brain/app.tsx at line 95, render Meetings only when meeting
upload is enabled and otherwise fall back to an enabled route; in
apps/web/lib/brain/routes.ts at line 13, reject persisted or hash-based meetings
navigation when disabled; and at line 33, disable keyboard key 8 unless meeting
upload is enabled.

In `@apps/web/components/brain/meetings.tsx`:
- Line 69: Update the meetingDate initialization in the meetings component to
derive the YYYY-MM-DD value from the user’s local date rather than
Date.toISOString(), preserving the existing state shape and setter behavior.
- Around line 102-115: Reset the per-result state alongside the new extraction
result in the existing extraction success flow: clear taught, cardErrors, and
decisionSupersedeConfirmed before rendering the new indexed cards. Keep the
assignee preselection logic unchanged, ensuring no state from the prior result
can apply to the new action items or decisions.
- Around line 128-140: Update postKnowledge to catch rejected fetch requests and
return the existing { ok: false, message } result with a suitable failure
message, allowing its callers to complete their cleanup. Preserve the current
response parsing and success behavior for HTTP responses.

In `@docs/protocols/meeting-miner.md`:
- Around line 12-18: In the webapp entry-point description, update the phrase
“calls this protocol describes” to “calls described by this protocol” (or the
equivalent grammatically correct wording), while leaving the surrounding content
unchanged.

In `@docs/REST_API.md`:
- Line 358: Update the unlabeled fenced code block in the REST API documentation
to include an appropriate language identifier, preferably text, while preserving
its existing contents.

In `@packages/core/src/meeting-extract.ts`:
- Around line 78-87: Update isValidActionItem to validate kind against the
declared supported kind union rather than accepting any string. Ensure the
action-item processing around the function’s kind handling rejects unsupported
values instead of allowing them to fall through and be relabeled as action-item.

In `@README.md`:
- Around line 194-196: Update the README feature-flag opt-in sentence to
explicitly show both assignments as V2_ACTION_ITEMS="true" and
V2_ORACLE_TASKS="true", replacing the ambiguous “both ="true"” wording.

---

Nitpick comments:
In `@apps/web/e2e/meetings.spec.ts`:
- Around line 3-11: The meeting E2E test is permanently skipped because its
feature flag is disabled in CI and it lacks deterministic extraction behavior.
In apps/web/e2e/meetings.spec.ts lines 3-11, add deterministic extraction
mocking so the meeting surface tests run without an LLM provider; in
.github/workflows/authed-e2e.yml line 179, set MEETING_UPLOAD_ENABLED=true for
both the web process and the Playwright command.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8acd7d27-06ef-4e19-948f-2f272459e587

📥 Commits

Reviewing files that changed from the base of the PR and between d636fa8 and f55fcef.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (25)
  • .env.example
  • .github/workflows/authed-e2e.yml
  • README.md
  • apps/web/app/api/me/route.ts
  • apps/web/app/api/meetings/extract/route.test.ts
  • apps/web/app/api/meetings/extract/route.ts
  • apps/web/components/brain/app.tsx
  • apps/web/components/brain/icons.tsx
  • apps/web/components/brain/meetings.tsx
  • apps/web/components/brain/shell.tsx
  • apps/web/e2e/meetings.spec.ts
  • apps/web/lib/brain/i18n.ts
  • apps/web/lib/brain/routes.ts
  • deploy/docker-compose.yml
  • docs/APPROACH.md
  • docs/GUIDELINES.md
  • docs/KNOWN_ISSUES.md
  • docs/MCP_TOOLS.md
  • docs/REST_API.md
  • docs/protocols/meeting-miner.md
  • packages/core/src/__tests__/env.test.ts
  • packages/core/src/__tests__/meeting-extract.test.ts
  • packages/core/src/env.ts
  • packages/core/src/index.ts
  • packages/core/src/meeting-extract.ts

Comment thread apps/web/app/api/meetings/extract/route.ts Outdated
Comment on lines +56 to +63
const store = await getRateLimitStore();
const rl = await rateLimitCheck(store, userId, meetingExtractLimit(), Date.now());
if (!rl.ok) {
return Response.json(
{ error: { code: "RATE_LIMITED", message: "Daily meeting-extraction limit reached." } },
{ status: 429 },
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Make rate-limit consumption atomic before using it for LLM cost control.

rateLimitCheck performs separate get and set operations. Parallel requests can all observe the same old bucket and exceed the daily cap. Add an atomic per-key consume operation for both Redis and memory stores, plus a concurrent-request test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/meetings/extract/route.ts` around lines 56 - 63, Replace the
non-atomic rateLimitCheck get/set flow used by the meeting extraction route with
an atomic per-key consume operation. Implement the consume primitive in both
Redis and in-memory rate-limit stores, update rateLimitCheck to use it while
preserving the existing limit response behavior, and add a concurrent-request
test proving parallel calls cannot exceed the daily cap.

Comment thread apps/web/app/api/meetings/extract/route.ts
Comment thread apps/web/app/api/meetings/extract/route.ts
autoskill: <Autoskill />,
sessions: <Sessions />,
decisions: <Decisions />,
meetings: <Meetings />,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Navigation hiding does not fully gate the meeting surface. Route hydration and keyboard navigation can still select the unconditionally rendered screen while the deployment flag is disabled.

  • apps/web/components/brain/app.tsx#L95-L95: guard the rendered screen using the loaded server capability and fall back to an enabled route.
  • apps/web/lib/brain/routes.ts#L13-L13: ensure persisted/hash-based meetings navigation is rejected when the capability is disabled.
  • apps/web/lib/brain/routes.ts#L33-L33: disable key 8 unless meeting upload is enabled.
📍 Affects 2 files
  • apps/web/components/brain/app.tsx#L95-L95 (this comment)
  • apps/web/lib/brain/routes.ts#L13-L13
  • apps/web/lib/brain/routes.ts#L33-L33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/components/brain/app.tsx` at line 95, Gate the meeting surface with
the loaded server capability: in apps/web/components/brain/app.tsx at line 95,
render Meetings only when meeting upload is enabled and otherwise fall back to
an enabled route; in apps/web/lib/brain/routes.ts at line 13, reject persisted
or hash-based meetings navigation when disabled; and at line 33, disable
keyboard key 8 unless meeting upload is enabled.

Comment thread apps/web/components/brain/meetings.tsx
Comment thread docs/protocols/meeting-miner.md
Comment thread docs/REST_API.md Outdated
Comment thread packages/core/src/meeting-extract.ts
Comment thread README.md Outdated
bejranonda and others added 4 commits July 18, 2026 07:55
…ns across users (I2)

buildRawProjectFilterV2's no-org-context branch bundles visibility='private'
into the same WHERE clause as 'project' with no owner check, so another
project member's explicitly private decision could surface as a
supersession candidate for anyone reviewing a meeting extraction. Add an
explicit exclusion local to this function's own query (does not touch
scope-filter.ts, which kra.ts/oracle.ts also depend on) — rows stay
project-wide for visibility='project' as designed, but visibility='private'
rows are now owner-only.

Test added first (db-guarded, same fixed-embedding pattern as the existing
findSupersessionCandidates test): seeds a private decision owned by a
different user in the same project, asserts it's excluded while a sibling
project-visibility row from that same user is still included.
…(I3)

POST /api/knowledge already rejects an action_item row's for:<email> tag
when the email isn't a real member of the resolved project's org, but PATCH
accepted patch.tags verbatim — an existing action_item's tags could be
edited to an arbitrary, never-validated for: assignee.

Extract the check into apps/web/lib/brain/assignee-validation.ts
(validateForTagAssignee) so both routes share one implementation and can't
drift apart. Kept it out of route.ts itself: Next.js App Router route
handlers are only sanctioned to export HTTP method handlers plus a small
set of segment-config values, and no other route.ts in this repo exports a
runtime function (only type-only interfaces, which are erased at compile
time) — a plain lib module avoids gambling on that build contract. PATCH
now runs the check only when patch.tags is being set on an action_item row,
leaving PATCH's behavior for other knowledge types and other fields
unchanged.

First test file for this route (apps/web/app/api/knowledge/[id]/route.test.ts),
following the db-guard + vi.spyOn(getCurrentUserId) pattern from
apps/web/app/api/knowledge/route.test.ts: covers the reject path (400
INVALID_ASSIGNEE, tags left unwritten), the accept path (real member,
200), and a non-action_item row to prove the check doesn't over-broaden.
…commendation)

Knowledge.instead exists in the schema and MCP's brain_teach_knowledge
already persists it, but POST /api/knowledge's createSchema had no `instead`
key at all — zod silently stripped it from any request body. meetings.tsx
displays d.instead ("Not: …") to the reviewer but teachDecision's payload
never included it, so the rejected alternative was shown then dropped the
moment Teach was clicked.

Add `instead: z.string().nullable().optional()` to createSchema and
`instead: body.instead ?? null` to the create-data mapping, mirroring the
existing `rationale` field's exact pattern in both places. Wire
`instead: d.instead || undefined` into meetings.tsx's teachDecision payload,
mirroring how `rationale` is already handled there.

Extends apps/web/app/api/knowledge/route.test.ts (Task 1's fixture) with a
test asserting a POST with `instead` set actually persists it to the row.
…le next/server

First real CI run on this branch (after the lockfile fix) failed both
apps/web route test files at module-load time, 0 tests collected:
"Cannot find module '.../next-auth/.../node_modules/next/server'". Root
cause: apps/web/auth.ts calls NextAuth(config) at module scope, and
next-auth@5's internals import next/server, which vitest's plain Node ESM
resolution can't resolve outside Next's own runtime. Every route test that
transitively imports @/lib/brain/auth hits this.

Fix: global vitest.setup.ts mocks @/auth's full export surface, one layer
below @/lib/brain/auth (which stays real — existing vi.spyOn(authLib,
"getCurrentUserId") usage is unaffected, since that spies on the real
module one layer up from this mock).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
apps/web/app/api/knowledge/route.ts (2)

202-214: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Return the updated row in the 201 payload.

created is still the pre-update object after tx.knowledge.update(...). As a result, the 201 response will have null for parentKnowledgeId even when supersession succeeds, as flagged in the previous review comment. Reassign the result of the update so the response correctly reflects the persisted state.

🐛 Proposed fix
-          await tx.knowledge.update({
-            where: { id: created.id },
-            data: { parentKnowledgeId: target.id },
-          });
+          const updated = await tx.knowledge.update({
+            where: { id: created.id },
+            data: { parentKnowledgeId: target.id },
+          });
+          return updated;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/knowledge/route.ts` around lines 202 - 214, Reassign the
result of the `tx.knowledge.update` call that sets `parentKnowledgeId` to
`target.id` back to `created`, so the returned 201 payload reflects the
persisted parent relationship. Keep the existing soft-delete update and response
flow unchanged.

85-101: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Apply tagPrefix across the complete result set before limiting.

Fetching a hardcapped 2,000 rows into memory and filtering them drops older matching rows that fall outside the initial pool. Since you cannot natively filter a string array prefix in Prisma, page through the candidates in batches to safely search the entire pool without causing memory exhaustion, as flagged in the previous review comment.

🐛 Proposed fix
-      const CANDIDATE_POOL_CAP = 2000;
-      const candidates = await db.knowledge.findMany({
-        where: finalWhere as never,
-        orderBy,
-        take: CANDIDATE_POOL_CAP,
-      });
-      const filtered = candidates.filter((r) => r.tags.some((t) => t.startsWith(tagPrefix)));
+      let skip = 0;
+      const take = 2000;
+      const filtered = [];
+      while (true) {
+        const candidates = await db.knowledge.findMany({
+          where: finalWhere as never,
+          orderBy,
+          skip,
+          take,
+        });
+        filtered.push(...candidates.filter((r) => r.tags.some((t) => t.startsWith(tagPrefix))));
+        if (candidates.length < take) break;
+        skip += take;
+      }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/knowledge/route.ts` around lines 85 - 101, Update the
tagPrefix branch in the knowledge query to search the complete result set before
applying limit: page through db.knowledge.findMany results in bounded batches
using stable pagination, filter each batch by tags starting with tagPrefix, and
accumulate only the matching records rather than loading the entire pool. Stop
when candidates are exhausted or enough matches are collected, then map the
first limit matches and report the full filtered total.
🧹 Nitpick comments (1)
apps/web/app/api/knowledge/[id]/route.test.ts (1)

37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test tag canonicalization by using mixed-case emails.

The test uses a purely lowercase email string generated via randomBytes. Injecting uppercase characters into the email definition will ensure that the canonicalization logic (once implemented) is rigorously tested against mixed-case inputs.

♻️ Proposed tweak
-      data: { email: `patch-real-member-${randomBytes(6).toString("hex")}`@test.local`` },
+      data: { email: `Patch-Real-Member-${randomBytes(6).toString("hex")}`@test.local`` },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/knowledge/`[id]/route.test.ts around lines 37 - 40, Update
the member email setup in the test’s db.user.create call to include uppercase
characters while retaining the randomized uniqueness, so the test exercises
canonicalization with a mixed-case email input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/app/api/knowledge/route.ts`:
- Line 128: Update the knowledge item fork logic in the POST handler and the
IMMUTABLE_FIELDS declaration in the [id] route to include instead alongside
rationale and ruleText. Mark instead immutable to block in-place edits, and copy
src.instead onto the newly forked row.

In `@apps/web/lib/brain/assignee-validation.ts`:
- Around line 20-48: Update validateForTagAssignee to inspect every for: tag,
canonicalize each assignee email to lowercase, validate all members, and return
the canonicalized tags alongside the success result. In
apps/web/app/api/knowledge/route.ts lines 160-165 and
apps/web/app/api/knowledge/[id]/route.ts lines 84-89, assign the returned
assigneeCheck.tags to body.tags and patch.tags respectively after successful
validation so persistence uses the canonicalized array.

---

Duplicate comments:
In `@apps/web/app/api/knowledge/route.ts`:
- Around line 202-214: Reassign the result of the `tx.knowledge.update` call
that sets `parentKnowledgeId` to `target.id` back to `created`, so the returned
201 payload reflects the persisted parent relationship. Keep the existing
soft-delete update and response flow unchanged.
- Around line 85-101: Update the tagPrefix branch in the knowledge query to
search the complete result set before applying limit: page through
db.knowledge.findMany results in bounded batches using stable pagination, filter
each batch by tags starting with tagPrefix, and accumulate only the matching
records rather than loading the entire pool. Stop when candidates are exhausted
or enough matches are collected, then map the first limit matches and report the
full filtered total.

---

Nitpick comments:
In `@apps/web/app/api/knowledge/`[id]/route.test.ts:
- Around line 37-40: Update the member email setup in the test’s db.user.create
call to include uppercase characters while retaining the randomized uniqueness,
so the test exercises canonicalization with a mixed-case email input.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5d2d4b4e-6a5d-441d-ae75-f2fce95def62

📥 Commits

Reviewing files that changed from the base of the PR and between f55fcef and c3dc4f4.

📒 Files selected for processing (12)
  • apps/web/app/api/knowledge/[id]/route.test.ts
  • apps/web/app/api/knowledge/[id]/route.ts
  • apps/web/app/api/knowledge/route.test.ts
  • apps/web/app/api/knowledge/route.ts
  • apps/web/app/api/meetings/extract/route.ts
  • apps/web/components/brain/meetings.tsx
  • apps/web/lib/brain/assignee-validation.ts
  • apps/web/vitest.config.ts
  • apps/web/vitest.setup.ts
  • deploy/docker-compose.yml
  • packages/core/src/__tests__/meeting-extract.test.ts
  • packages/core/src/meeting-extract.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • apps/web/vitest.config.ts
  • packages/core/src/tests/meeting-extract.test.ts
  • deploy/docker-compose.yml
  • apps/web/app/api/knowledge/route.test.ts
  • apps/web/components/brain/meetings.tsx

Comment thread apps/web/app/api/knowledge/route.ts
Comment on lines +20 to +48
export async function validateForTagAssignee(
tags: string[],
resolvedProjectId: string | null,
): Promise<{ ok: true } | { ok: false; status: number; body: unknown }> {
const forTag = tags.find((t) => t.startsWith("for:"));
if (!forTag) return { ok: true };
const assigneeEmail = forTag.slice(4).toLowerCase();
const project = resolvedProjectId
? await db.project.findUnique({
where: { id: resolvedProjectId },
select: { organizationId: true },
})
: null;
const members = project ? await listOrgMembers(db, project.organizationId) : [];
const isMember = members.some((m) => m.email.toLowerCase() === assigneeEmail);
if (!isMember) {
return {
ok: false,
status: 400,
body: {
error: {
code: "INVALID_ASSIGNEE",
message: `${assigneeEmail} is not a member of this project's organization.`,
},
},
};
}
return { ok: true };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Canonicalize and validate every for: tag before persistence.

All three sites share a flawed tag-validation approach that only inspects the first for: tag and persists unnormalized mixed-case emails, bypassing assignment validation for secondary tags and breaking downstream assignee lookups (as flagged in a previous review comment).

  • apps/web/lib/brain/assignee-validation.ts#L20-L48: Update the function to find and validate all for: tags, then replace the original tags with their lowercase canonical forms and return the modified array.
  • apps/web/app/api/knowledge/route.ts#L160-L165: Assign body.tags = assigneeCheck.tags upon successful validation to persist the canonicalized array.
  • apps/web/app/api/knowledge/[id]/route.ts#L84-L89: Assign patch.tags = assigneeCheck.tags upon successful validation to persist the canonicalized array.
🐛 Proposed fixes

In apps/web/lib/brain/assignee-validation.ts:

 export async function validateForTagAssignee(
   tags: string[],
   resolvedProjectId: string | null,
-): Promise<{ ok: true } | { ok: false; status: number; body: unknown }> {
-  const forTag = tags.find((t) => t.startsWith("for:"));
-  if (!forTag) return { ok: true };
-  const assigneeEmail = forTag.slice(4).toLowerCase();
+): Promise<{ ok: true; tags: string[] } | { ok: false; status: number; body: unknown }> {
+  const forTags = tags.filter((t) => t.startsWith("for:"));
+  if (forTags.length === 0) return { ok: true, tags };
+
+  const assigneeEmails = forTags.map((t) => t.slice(4).trim().toLowerCase());
   const project = resolvedProjectId
     ? await db.project.findUnique({
         where: { id: resolvedProjectId },
         select: { organizationId: true },
       })
     : null;
   const members = project ? await listOrgMembers(db, project.organizationId) : [];
-  const isMember = members.some((m) => m.email.toLowerCase() === assigneeEmail);
-  if (!isMember) {
+  const memberEmails = new Set(members.map((m) => m.email.toLowerCase()));
+
+  const invalidAssignee = assigneeEmails.find((email) => !memberEmails.has(email));
+  if (invalidAssignee) {
     return {
       ok: false,
       status: 400,
       body: {
         error: {
           code: "INVALID_ASSIGNEE",
-          message: `${assigneeEmail} is not a member of this project's organization.`,
+          message: `${invalidAssignee} is not a member of this project's organization.`,
         },
       },
     };
   }
-  return { ok: true };
+
+  const canonicalTags = tags.map((tag) =>
+    tag.startsWith("for:") ? `for:${tag.slice(4).trim().toLowerCase()}` : tag,
+  );
+  return { ok: true, tags: canonicalTags };
 }

In apps/web/app/api/knowledge/route.ts (POST):

     if (body.type === "action_item") {
       const assigneeCheck = await validateForTagAssignee(body.tags, resolvedProjectId);
       if (!assigneeCheck.ok) {
         return Response.json(assigneeCheck.body, { status: assigneeCheck.status });
       }
+      body.tags = assigneeCheck.tags;
     }

In apps/web/app/api/knowledge/[id]/route.ts (PATCH):

     if (patch.tags !== undefined && check.row.type === "action_item") {
       const assigneeCheck = await validateForTagAssignee(patch.tags, check.row.ownerProjectId);
       if (!assigneeCheck.ok) {
         return Response.json(assigneeCheck.body, { status: assigneeCheck.status });
       }
+      patch.tags = assigneeCheck.tags;
     }
📍 Affects 3 files
  • apps/web/lib/brain/assignee-validation.ts#L20-L48 (this comment)
  • apps/web/app/api/knowledge/route.ts#L160-L165
  • apps/web/app/api/knowledge/[id]/route.ts#L84-L89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/lib/brain/assignee-validation.ts` around lines 20 - 48, Update
validateForTagAssignee to inspect every for: tag, canonicalize each assignee
email to lowercase, validate all members, and return the canonicalized tags
alongside the success result. In apps/web/app/api/knowledge/route.ts lines
160-165 and apps/web/app/api/knowledge/[id]/route.ts lines 84-89, assign the
returned assigneeCheck.tags to body.tags and patch.tags respectively after
successful validation so persistence uses the canonicalized array.

bejranonda and others added 7 commits July 18, 2026 15:15
…ignal

Second real CI run (after the next-auth module-load fix) got past module
loading and ran 16 of 19 web route tests; the 3 that still failed all
returned 500 where 200/429 was expected. Root cause: getActiveProject
(apps/web/lib/brain/active-project.ts) calls `await cookies()`
unconditionally, and Next's cookies() throws outside a request-scoped
AsyncLocalStorage context — which a route handler invoked directly as a
plain function from a vitest test doesn't have. This was flagged as a
residual, unconfirmed risk in the prior fix's own report; this CI run
confirmed it.

Mocks only `cookies` (via importOriginal, preserving every other
next/headers export) to resolve with a no-op get() — exactly the
no-cookie-set fallback path every existing route test already relies on.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…flag wording)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…validation, instead immutability, paginated tagPrefix scan

CodeRabbit findings, verified live on this branch:
- POST /api/knowledge's create+supersede transaction returned the
  pre-update `created` row instead of the parentKnowledgeId-linked update,
  so a successful supersede still showed parentKnowledgeId: null in the
  201 response body.
- validateForTagAssignee only checked the first `for:` tag and never
  canonicalized the persisted tag's case, even though
  action-items.ts's assignee lookup is an exact-match on the lowercased
  email — a mixed-case for: tag would silently never surface to its
  assignee. Now validates every for: tag and both call sites (POST
  create, PATCH) persist the canonicalized array.
- `instead` was missing from IMMUTABLE_FIELDS and the fork-copy logic,
  unlike its sibling `rationale`.
- GET ?tagPrefix filtered a single hard-capped 2000-row pool, silently
  dropping a real match beyond that boundary. Now pages through bounded
  batches (2000/batch, 10-batch ceiling) so a match is found regardless
  of position, with an id tiebreaker added to keep pagination stable
  across batches.
- The "different project" supersession-exclusion test called
  ensureDefaultProject a second time, which is idempotent per user and
  silently returned the same project — the test never exercised
  cross-project exclusion. Now creates a genuinely distinct project via
  ensureNamedProject, with a hard assertion on id distinctness, and
  deletes that project again immediately after use to avoid destabilizing
  getActiveProject's unordered no-cookie fallback for later tests in the
  same file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…persession-enrichment concurrency

CodeRabbit findings, verified live on this branch:
- A malformed JSON body made req.json() throw a raw SyntaxError (not a
  ZodError), falling through to a generic 500 instead of a proper 400.
  req.json() is now awaited in its own try/catch, returning
  {error:{code:"INVALID_REQUEST",...}} at 400 before schema validation
  runs.
- The decision-supersession enrichment step ran an unrestricted
  Promise.all, one embedding-provider call per extracted decision, with
  no cap — a cost/latency concern for an unusually large transcript.
  Capped at the first 20 decisions; every decision still appears in the
  response, decisions beyond the cap just get supersedes: null instead
  of an enrichment lookup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…tch network fetch failures, use local date

CodeRabbit findings, verified live on this branch:
- handleExtract's success path never cleared taught/teaching/cardErrors/
  decisionSupersedeConfirmed from a prior extraction, so a second
  transcript's cards at the same index-derived keys (d-0, a-0, …) could
  inherit stale state — e.g. a brand-new decision rendering as
  "✓ Taught" without ever being submitted. Now reset alongside
  setResult/setMode.
- postKnowledge only handled non-ok HTTP responses; a network-level
  fetch() rejection (offline, CORS, DNS) propagated uncaught to
  teachDecision/teachActionItem, neither of which wrap the call in their
  own try/catch. Now wrapped, returning a readable network-failure
  message that flows through the existing cleanup path.
- meetingDate's initial value used toISOString().slice(0,10), which
  shifts to UTC first and can show the wrong calendar date near
  midnight off-UTC. Now built from local date components.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
CodeRabbit finding, verified live on this branch: the predicate only
checked typeof r["kind"] !== "string", so a malformed LLM response like
kind: "task" passed validation and was then silently relabeled to
"action-item" by parseExtractionResponse's mapping, instead of being
dropped. Now rejects any kind outside "action-item" | "open-question"
(undefined is still allowed, matching the existing optional-field
pattern for the rest of the predicate).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM
…SUES row

CodeRabbit flagged rateLimitCheck's get-then-set race during meeting-
transcript-upload review; the fix touches shared infra (both Redis and
in-memory rate-limit stores) other pre-existing features already depend
on, so it's out of scope for this branch. Recording the deferral with its
trigger-to-revisit condition rather than letting the finding evaporate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfujUMcYWzRkwHmG8RYTDM

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/app/api/knowledge/route.test.ts (1)

106-158: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Guarantee cleanup of the second project with finally.

If POST or any assertion throws before Line 158, the project remains in the shared database and can make later active-project resolution nondeterministic. Wrap the test body after project creation in try/finally.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/knowledge/route.test.ts` around lines 106 - 158, Wrap the
test body after ensureNamedProject creates otherProjectId in a try/finally
block, placing the existing project deletion in finally. Preserve all request,
assertions, and knowledge cleanup behavior while guaranteeing the extra project
is removed if POST or any assertion fails.
🧹 Nitpick comments (1)
apps/web/app/api/meetings/extract/route.test.ts (1)

184-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Strengthen the supersession enrichment assertion.

Currently, findCandidatesMock resolves to an empty array [], causing the first 20 decisions to have supersedes: null—which is identical to the behavior of the uncapped decisions. Modifying the mock to return a mock candidate will explicitly prove that the first 20 decisions actually received the enrichment from the mock, preventing false positives if the enrichment logic is accidentally altered.

♻️ Proposed refactor
-    findCandidatesMock.mockResolvedValue([]);
+    findCandidatesMock.mockResolvedValue([{ id: "fake-id" }] as any);

     const req = new Request("http://test.local/api/meetings/extract", {
       method: "POST",
       body: JSON.stringify({ transcript: "a meeting with an unusually large decision list" }),
     });
     const res = await POST(req);
     expect(res.status).toBe(200);

-    const body = (await res.json()) as { decisions: Array<{ supersedes: unknown }> };
+    const body = (await res.json()) as { decisions: Array<{ supersedes: { id: string } | null }> };
     expect(body.decisions).toHaveLength(25);
     expect(findCandidatesMock).toHaveBeenCalledTimes(20);
     expect(body.decisions.slice(20).every((d) => d.supersedes === null)).toBe(true);
-    expect(body.decisions.slice(0, 20).every((d) => d.supersedes === null)).toBe(true); // mock resolves []
+    expect(body.decisions.slice(0, 20).every((d) => d.supersedes?.id === "fake-id")).toBe(true);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/api/meetings/extract/route.test.ts` around lines 184 - 197,
Update the large decision-list test around findCandidatesMock so it resolves a
mock candidate that produces a non-null supersedes value for enriched decisions.
Assert the first 20 decisions reflect that enrichment, while the decisions
beyond the 20-item cap retain supersedes: null, and preserve the existing
call-count assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@apps/web/app/api/knowledge/route.test.ts`:
- Around line 106-158: Wrap the test body after ensureNamedProject creates
otherProjectId in a try/finally block, placing the existing project deletion in
finally. Preserve all request, assertions, and knowledge cleanup behavior while
guaranteeing the extra project is removed if POST or any assertion fails.

---

Nitpick comments:
In `@apps/web/app/api/meetings/extract/route.test.ts`:
- Around line 184-197: Update the large decision-list test around
findCandidatesMock so it resolves a mock candidate that produces a non-null
supersedes value for enriched decisions. Assert the first 20 decisions reflect
that enrichment, while the decisions beyond the 20-item cap retain supersedes:
null, and preserve the existing call-count assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 17124681-9566-4a69-9b5b-43ff04f3589b

📥 Commits

Reviewing files that changed from the base of the PR and between c3dc4f4 and 58dd86a.

📒 Files selected for processing (13)
  • README.md
  • apps/web/app/api/knowledge/[id]/route.ts
  • apps/web/app/api/knowledge/route.test.ts
  • apps/web/app/api/knowledge/route.ts
  • apps/web/app/api/meetings/extract/route.test.ts
  • apps/web/app/api/meetings/extract/route.ts
  • apps/web/components/brain/meetings.tsx
  • apps/web/lib/brain/assignee-validation.ts
  • apps/web/vitest.setup.ts
  • docs/REST_API.md
  • docs/protocols/meeting-miner.md
  • packages/core/src/__tests__/meeting-extract.test.ts
  • packages/core/src/meeting-extract.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/web/app/api/meetings/extract/route.ts
  • packages/core/src/tests/meeting-extract.test.ts
  • docs/REST_API.md
  • README.md
  • apps/web/app/api/knowledge/route.ts
  • packages/core/src/meeting-extract.ts
  • apps/web/components/brain/meetings.tsx

@bejranonda bejranonda changed the title WIP: meeting transcript upload — Tasks 1-2 (foundation, new vitest infra) feat(web): meeting transcript upload — paste, extract, review, teach (flag-gated, dark) Jul 18, 2026
@bejranonda
bejranonda merged commit 66b03f4 into main Jul 18, 2026
6 checks passed
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.

1 participant