Skip to content

feat(meetings): translate the transcript line by line - #5739

Merged
bolichen97 merged 1 commit into
kirodotdev:mainfrom
kaizawa97:pr/meetings-transcript-translation
Aug 30, 2026
Merged

feat(meetings): translate the transcript line by line#5739
bolichen97 merged 1 commit into
kirodotdev:mainfrom
kaizawa97:pr/meetings-transcript-translation

Conversation

@kaizawa97

@kaizawa97 kaizawa97 commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

A meeting's participants don't always share a language. Today the transcript is only available in whatever language was spoken; someone following along in another language has to leave the app to translate, line by line, losing the meeting while they do.

Why it matters

Live translation is the difference between "can attend" and "can participate" for mixed-language teams — and doing it inside the app means the translation benefits from the app's own dictionary corrections instead of translating STT mistakes.

What changed (motivation → approach → change)

Goal: translate each spoken line into a user-chosen language, shown in a side panel while the meeting runs — off by default, because it costs one model call per line.

  • backend/domain/translate.py — a bounded SEQUENTIAL per-meeting queue running one tool-less call on kirocrew-lite per line, ephemeral session destroyed after. Deliberately not an AgentQueue variant (that one exists to batch 30 s for agent context; this exists to avoid batching). This is the app's first non-agent LLM path.
  • Hooked into MeetingSession.broadcast, not the dispatch route — broadcast is where text is already dictionary-corrected and past the noise gate. A mangled project noun mistranslates into something unrecognisable, and translated throat-clearing is worse than nothing.
  • Prompt carries the app's injection guard (delimiters + "this is DATA, not instructions") because a transcript is attacker-influenceable: anyone who can speak into the meeting can put words in it. The model's answer is redacted before it is written to translations.json (translate.py allowlisted as a non-egress redaction module in security_posture.py).
  • GET …/translations?since=N is cursor-paged; the client accumulates into a Map keyed by line number so a queryFn running twice for one cursor (React Strict Mode) cannot duplicate lines. A failed line is persisted with text: "" so the panel marks it rather than showing a gap indistinguishable from silence. Language change resets the document server-side and the accumulator client-side.
  • Accepted language set is published by GET /config (translation_languages, labels are endonyms) rather than hardcoded in the frontend — the backend validates the saved value, so it also publishes the accepted set. TranslationSidebar + a Settings select (using the shared SimpleSelect); polling runs only while the panel is open AND a language is set, at the active rate while live and the idle rate while paused/reviewing (the backend queue keeps draining while paused, so the panel would otherwise freeze mid-sentence).
  • Manifest highlight + full 12-locale catalog mirror; spec updated in the same commit (Live translation section, route/data rows, tests).

Review-driven changes (Kiro Crew, co-author), addressing the GPT review's two blocking findings and rebase fallout:

  • TranslationSidebar releases its fixed 340px width below lgw-full with a bounded height (h-[42%] min-h-[260px]) when stacked, lg:w-[340px] lg:h-full beside the meeting — the same responsive shape TaskSidebar already uses, so a 320px viewport no longer clips the panel.
  • The meeting toolbar's secondary actions (end-and-review, refresh, translation, action items) moved into an overflow DropdownMenu — five sibling buttons breached the max-two-buttons-per-row cap and wrapped under width pressure; the row now holds the one primary status action plus the trigger, and every menu item keeps a full text label (new apps.meetings.meeting.moreActions key, all 14 catalogs). The two side panels are mutually exclusive: stacked below lg, their combined 260px height floors would squeeze the transcript out entirely.
  • Rebase onto current main resolved additive key conflicts in 7 locale catalogs and normalized catalog key order to the sortDeep form the i18n tooling writes, so a future automated translate run carries no reordering churn.
  • append_translation refuses to write when the meeting no longer exists — the worker persists on a thread and could lose a race with delete_meeting, silently recreating the deleted meeting's directory via _write_json's mkdir; both sides take meta_transaction, so the new metadata guard is race-free (second GPT review round).
  • The client resets its translation cursor when the server's document language changes — a replaced document restarts numbering at zero, so a cursor advanced against the old document filtered out every initial line of the new language; the accumulator is now keyed on the last observed server language (not live config, which would wipe it every poll while a running session and config disagree) and refetches from zero on change (second GPT review round).
  • website/scripts/capture-meetings-translation.mjs — a capture harness (real SPA, deterministic API fixtures, following capture-meetings-delete.mjs) that verifies the toolbar cap and the 320px sidebar bounding box, and produces the screenshots below.

Tests

  • test/test_meetings_translation.py (52 tests) — the injection guard, the bounded queue (never blocks, never raises, drops with a count when the backlog fills), off-by-default, unknown-language-resolves-to-off, redaction of the model's answer, the cursor endpoint, and the delete-race guard (a worker write racing delete_meeting cannot recreate the deleted directory).
  • website/src/test/MeetingsTranslation.test.tsx — the Map-keyed accumulation (Strict-Mode idempotence), the reset on language change, the enabled-gating of the poll, the paused/reviewing idle-poll ladder, the two-control toolbar cap (secondary actions live in the overflow menu, not as row buttons), the side-panel mutual exclusion, the responsive sidebar shape (releases 340px below lg, bounded height, divider turns with the layout), and the server-language cursor reset (refetch from zero, keyed on observed server language).

Local runs: backend isort/flake8/mypy (1167 files) and black ratchet green; test_meetings_translation.py 51 passed and the meetings test family 606 passed; website tsc, eslint, i18n:check, and the full vitest suite (25,907 passed) green.

Manual verification

Exercised end-to-end against a live harness gateway (fake ACP backend):

  • Set the target language to Japanese via PUT /config (translation_language), started a meeting, and dispatched two English lines.
  • Polled GET …/translations?since=0 until both lines carried translated text produced by the per-line queue (one ephemeral, tool-less session per line), and confirmed the reported language surfaced as ja / 日本語.
  • Confirmed the feature stays off until a language is set (no queue work, no translations.json).

The screenshots below come from the capture harness (real production SPA, stubbed API): the overflow menu open on a live meeting, the sidebar beside the meeting at lg, and the stacked bounded panel at a 320px viewport. Still pending before merge sign-off: a pass against a production translation model — the harness backend returns a canned reply, so this proves the pipeline (queue → ephemeral session → cursor-paged endpoint → accumulator), not translation quality.

Screenshots / video

Live meeting, dark theme, from scripts/capture-meetings-translation.mjs.

Toolbar overflow menu open: End and review, Refresh, Translation, Action items as labelled menu items; the row holds Pause plus the trigger

Translation sidebar at 340px beside the meeting: 日本語 badge, source/translation line pairs

320px viewport: the panel stacks with a bounded height instead of clipping

Translation panel stacked below the transcript at a 320px viewport, source/translation pairs readable, toolbar within the two-control cap

Related Issues

Part of the meetings feature stack split from the feat/meetnote branch. A follow-up PR (a per-meeting note the user owns) depends on this one and will be submitted once this merges.

no linked issue: feature work from the meetings stack, no filed issue tracks it.

Checklist

  • Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@kaizawa97
kaizawa97 requested a review from a team August 25, 2026 01:32
@kaizawa97
kaizawa97 requested a review from a team as a code owner August 25, 2026 01:32
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running labels Aug 25, 2026
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 106b432360871a38d57c8af70ebde55d0cbe2b42 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 106b432

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 106b432360871a38d57c8af70ebde55d0cbe2b42 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts verified. Composing the review now.

First-Principles-Verdict: CONCERNS

A third hand-rolled ephemeral kirocrew-lite one-shot ships — and the spec anoints it the reuse point — while llm_helpers.background_turn exists precisely because hand-rolled lifecycles skip usage accounting.

What this change ships

Intent: let someone follow a meeting held in a language they don't speak, translated line by line in a side panel. ADDITION.

  1. Settings gains a "Live translation" language picker, off by default — justified
  2. A side panel shows each spoken line with its translation, live — justified
  3. End-and-review, Refresh, Action items moved into an overflow menu — declared move, derived from blocking max-two-buttons-per-row (website/AUTOSDE.yaml:230)
  4. Opening one side panel now closes the other — declared, derived from stacked height floors
  5. New GET …/translations?since= endpoint + per-meeting translations.json — justified
  6. New config key translation_language; accepted set published by GET /config — justified
  7. Failed lines render as marked gaps; overflow drops oldest and says so — justified
  8. A late translation write can no longer resurrect a deleted meeting — justified
  9. Capture harness + 3 committed PNGs — follows repo convention (temp-screenshots/ holds ~150 committed sibling dirs)
  10. App Store card gains a translation highlight, mirrored to all catalogs — declared, i18n-gate derived

Watch

  • run_oneshot_translation is the third spelling of "fresh ephemeral kirocrew-lite, REJECT_ALL, destroy after" — count 3: issue_radar/backend/routes.py:1985 (_run_oneshot_model), workflows/service.py:704, now translate.py. llm_helpers.background_turn's own docstring names the harm of hand-rolling: spend "reached the provider's bill without ever reaching the usage store." At one model call per spoken line this may be the app's largest background spender, and none of it is accounted. The new spec sentence "anything else needing a quick model call should reuse it" points future callers at the app-local copy instead of llm_helpers, where the family (background_turn, run_bg_oneliner) already lives.

Subtractions

  • Delete TranslationQueue.drain() — zero production consumers (5 call sites, all in test_meetings_translation.py; teardown goes cancel_translationsclear()), and its docstring claims a teardown role ("Used at meeting teardown") that cancel_translations' docstring explicitly rejects. Tests can await the worker task directly.
  • Drop TranslationQueue.enabled and the if not self.enabled branch — the only production constructor (MeetingSession.__post_init__) already gates on TRANSLATION_LANG_CODES, so a disabled queue never exists outside tests (consumers of enabled: 2, both test asserts). Require a non-empty language instead.

[FIRST-PRINCIPLES-REVIEWED] 106b432

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 106b432360871a38d57c8af70ebde55d0cbe2b42 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Per-line ephemeral kiro-cli sessions stack process-spawn overhead onto a latency-constrained loop, and real-model pacing is still unvalidated — otherwise a sound, well-contained design.

Watch

  • The hot loop spawns, handshakes, and destroys a full kirocrew-lite session per spoken line, sequentially (run_oneshot_translation: get_or_createdestroy per call), so per-line wall-clock is spawn + model latency compounded. The PR itself says the only end-to-end run used a canned-reply harness ("Still pending before merge sign-off: a pass against a production translation model"). If real per-line cost exceeds speaking pace, the 40-line backlog drops continuously and the panel becomes a permanent "Catching up… / Some lines were skipped" — degradation is graceful, but the feature's core promise (keeping up live) is the one property not yet demonstrated. Do the production-model pass before sign-off, as planned.

Suggestions

  • If real latency proves inadequate, a per-meeting persistent tool-less session (destroyed in cancel_all) removes the spawn/handshake from every line; the queue is already sequential so ordering holds, and the tool-less + redact posture bounds injection carry-over.
  • The Settings help doesn't say a language change takes effect at the next meeting (__post_init__ fixes it for the session); one sentence in translationHelp prevents "I changed it and nothing happened".

[DESIGN-REVIEWED] 106b432

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 106b432360871a38d57c8af70ebde55d0cbe2b42 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

Both the meeting's mandatory closing action and the new translation feature now live behind a hidden "…" menu — findability, not the panel itself, is the risk.

Watch

  • "End and review" — the only way to end a meeting and reach the review/filing flow — moves from a visible labeled danger button into the icon-only overflow (DropdownMenuItem onSelect={actions.review}). Every meeting hits this once, and existing users' muscle memory points at a button that is gone; first-timers must guess the "…" hides the exit. Smallest fix: keep End and review in the row beside the status action (Pause/End is one status pair) and overflow only refresh/panels.
  • Translation is invisible until configured: {translation.language && (<DropdownMenuItem…toggleTranslation)} hides the item when off, and nothing in the meeting view points at Settings. A user who saw the store highlight ("Read the transcript back… in another language") finds no trace of it at the point of need. Fix: always render the item; when unset, route to Settings (or disable with a "set a language in Settings" hint).
  • "Translation" and "Action items" menu items are toggles that also silently close each other, but expose no open/closed state visually or to AT — a reader cannot tell whether selecting opens, closes, or switches. Fix: DropdownMenuCheckboxItem (or checked indicator) reflecting each panel's open state.

[UX-REVIEWED] 106b432

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 106b432360871a38d57c8af70ebde55d0cbe2b42 via the fork AI-review pipeline; updated in place on each push.

Review details

I've verified both candidates against the actual code.

Candidate 1 — confirmed. handle_dispatch_text (agents.py:344) builds line = f"{k.CHAT_PREFIX} {transcript_text}" for typed/chat lines and calls session.broadcast(line). The added enqueue in broadcast (session.py:483) passes that prefixed text straight to self.translations.enqueue(...), which stores it as source and feeds it to the prompt. Meanwhile the transcript itself stores the clean transcript_text (agents.py:333). TranslationSidebar renders {line.source} verbatim. So the [chat] marker leaks into the panel's source column and into the translation prompt for typed lines. Real, observable, but minor UX — advisory, not blocking.

Candidate 2 — dropped. The (c) outcome is "the model may follow the injected instruction," a "might," not a definite wrong outcome; the guard is the standard best-effort delimiter+DATA pattern the rest of the app uses, output is redacted, and tools are blocked by REJECT_ALL. Candidate's own confidence is low. Fails the bar.

No new Step 2 findings meet the bar.

Advisory only — the internal [chat] marker leaks into the live-translation panel for typed lines.

FINDING — src/kiro_crew/apps/builtins/meetings/backend/domain/session.py:483 — typed/chat dispatches reach broadcast as f"{k.CHAT_PREFIX} {transcript_text}", and self.translations.enqueue(text) stores that prefixed string as source and sends it to the translator, so the panel shows "[chat] …" (and asks the model to translate the marker) while the transcript keeps the clean transcript_text → Fix: strip k.CHAT_PREFIX before enqueueing, or enqueue the un-prefixed transcript_text.

[OPUS-REVIEWED] 106b432

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Aug 25, 2026
@chenmingwei23 chenmingwei23 added the needs-pr-triage PR scanner: awaiting automated triage label Aug 29, 2026
@NicholasRBowers NicholasRBowers added drive-to-green PR claimed by drive-to-green pipeline and removed needs-pr-triage PR scanner: awaiting automated triage labels Aug 29, 2026
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: This PR has been sitting with failing CI and no recent author activity. I've assessed the blockers and they appear resolvable — I'll push fixes directly to this branch as a co-author.

Assessment: Merge conflict is confined to 7 i18n locale JSON files (additive-key resolution), and the GPT review's two blocking UI findings come with prescribed fixes (responsive TranslationSidebar width; fold secondary MeetingView toolbar actions into an overflow menu). Plan: rebase, resolve locale conflicts, apply both prescribed UI fixes + spec updates, refresh screenshots, and disposition the advisory lanes.

If you'd prefer I don't touch this PR, add the pr-no-autofix label.

@NicholasRBowers
NicholasRBowers force-pushed the pr/meetings-transcript-translation branch from ad7b568 to 197b00e Compare August 29, 2026 04:49
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 29, 2026
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Pushed the drive-to-green fixes as a new head (197b00e3ba36), single commit, original author preserved with a Kiro Crew co-author trailer. Every change, and why:

1. Rebase onto current main — the merge conflict was confined to 7 locale catalogs (de, en-XA, en, es, fr, it, pt), all the same additive-key shape: main added use_case_1/configuration_1 to the meetings manifest section while this PR added highlight_6. Resolved by keeping both sides' keys; all 7 files verified as valid JSON and i18n:check passes.

2. GPT blocking finding (a) — fixed sidebar clips at 320px (TranslationSidebar.tsx): replaced the fixed w-[340px] with the exact responsive shape TaskSidebar already uses in this app — w-full h-[42%] min-h-[260px] border-t stacked below lg, lg:w-[340px] lg:h-full lg:border-t-0 lg:border-l beside the meeting.

3. GPT blocking finding (b) — toolbar exceeds the two-action cap (MeetingView.tsx): the row now holds the one primary status action (Start/Pause/Resume) plus a single overflow DropdownMenu trigger; End and review, Refresh, Translation, and Action items are menu items with full text labels — following CronRowActions.tsx and issue-radar's DetailOverflowMenu (including moving a significant action into the menu, as that pane's Close does). New apps.meetings.meeting.moreActions label in all 14 catalogs, reusing each locale's existing "More actions" translation.

4. Side-panel mutual exclusion (caught by a local pre-push Opus-mirror review): with the translation panel now stacking below lg, opening both it and the task sidebar put two flex-none min-h-[260px] children inside an overflow-hidden column — 520px of floor that squeezes the transcript out of a short viewport. Opening either panel now closes the other.

5. Translation poll ladder (same local review): translationQuery stopped polling entirely for any non-active status, but pausing does not clear the backend queue — the worker keeps draining and persisting lines — so the panel froze mid-sentence and never rendered the tail. It now mirrors the sibling outputs/transcript queries: active rate while live, idle rate while paused/reviewing.

6. Catalog key-order normalization (same local review): the three settings.translation* keys and the top-level translation section landed unsorted in 11 of 13 catalogs (rebase-resolution drift; ja/ko were already sorted). Sorted them to the sortDeep form i18n-translate.mjs writes, so the next automated translate run carries no reordering churn; en-XA regenerated via gen-pseudolocale.mjs.

7. Tests + evidence: MeetingsTranslation.test.tsx gains pins for the toolbar cap, the mutual exclusion, the idle-poll ladder, and the responsive sidebar shape (all verified red against the unfixed sources); new scripts/capture-meetings-translation.mjs harness (following capture-meetings-delete.mjs) produces the PR-body screenshots and asserts the toolbar cap and the 320px bounding box at capture time.

Local gates on the final head: backend isort/flake8/mypy/black-ratchet green, test_meetings_translation.py 51 passed + meetings family 606 passed; frontend tsc, eslint, i18n:check, full vitest 25,907 passed.

@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Dispositions for the GPT 5.6 Review findings on ad7b5685:

  • fixedTranslationSidebar.tsx:53 fixed w-[340px] sidebar breaks narrow viewports.

    320px viewport -> open translation -> 340px child is clipped by the meeting shell.

    Applied the prescribed fix in 197b00e3ba36: the aside is now w-full with a bounded height (h-[42%] min-h-[260px]) when stacked below lg, and lg:w-[340px] lg:h-full from lg up — byte-for-byte the responsive shape TaskSidebar already uses in this app. The capture harness asserts the 320px bounding box (width ≤ viewport, not clipped off-canvas) at screenshot time, and MeetingsTranslation.test.tsx pins the class shape.

  • fixedMeetingView.tsx:204 toolbar exceeds the two-action cap.

    Configured translation -> active meeting -> five sibling actions render and wrap under width pressure.

    Applied the prescribed fix in 197b00e3ba36: translation and the other secondary actions (end-and-review, refresh, action items) moved into an overflow DropdownMenu; the row holds the one primary status action plus the trigger, which counts as one control. MeetingsTranslation.test.tsx pins that each secondary action renders as an onSelect menu item and not an onClick row button.

@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Dispositions for the Design Review concerns on ad7b5685:

  • rebutted (documented limitation, per the PR's own framing) — config-as-live vs fixed-per-meeting mismatch.

    The queue is built once in __post_init__ ("the language is fixed for the meeting"), but MeetingView shows the toggle from config?.translation_language ...

    The commit message and PR body state the contract: "The target language is a per-meeting setting" — a mid-meeting Settings change deliberately takes effect at the next meeting, matching how the rest of a meeting's configuration (agents, preset) binds at start. Re-plumbing the live session to rebuild its queue on PUT /config is a design change beyond this drive's scope (the drive's mandate is to fix the two blocking findings and the conflict, not to redesign the seam), and the one-poll-wipe symptom is bounded: the panel still renders each incremental page, and ending/starting a meeting recovers fully. Left as a documented limitation; if the maintainer wants the "takes effect next meeting" hint or live rebinding, that is a follow-up decision on the feature's contract, not a defect fix.

  • needs-a-decision — live-model latency pass as a merge gate.

    The promised live-meeting verification should be treated as a merge gate, not a follow-up.

    The PR body already declares this pending: "Still pending before merge sign-off: a pass against a production translation model." Whether that pass gates merge is the maintainer's call — flagging it here so the question is answered where it can be, rather than filed as an unactionable issue. The reviewer's own mitigation (reuse one tool-less session per meeting if spawn cost dominates) is recorded in-thread for whoever runs the pass.

@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Dispositions for the UX Review concerns on ad7b5685:

  • rebutted (documented limitation, per the PR's own framing) — settings don't reach a running meeting, and nothing says so.

    A user who discovers the feature during a meeting — the most likely first-use path — enables it, opens the panel, and reads "Lines appear here as they are spoken." while nothing can ever arrive ...

    Same root as the Design lane's first concern: the language is a per-meeting setting by declared contract, binding at meeting start like the rest of a meeting's configuration. The proposed copy fix ("Takes effect when the next meeting starts." in translationHelp and the empty state) is reasonable but changes 14 locale catalogs and the settings/panel copy on a seam whose final shape is the maintainer's call (hint text vs. live rebinding vs. gating the toggle on the session's actual state) — this drive deliberately does not pre-empt that choice. Left as a documented limitation with the options recorded here.

  • fixed — no narrow-width behavior, unlike its sibling.

    TranslationSidebar is a fixed w-[340px] border-l with no responsive variant, while TaskSidebar stacks below lg ...

    Fixed in 197b00e3ba36 exactly as suggested: the panel now reuses TaskSidebar's responsive classes (w-full h-[42%] min-h-[260px] border-t below lg, lg:w-[340px] lg:h-full lg:border-l), and the "both panels open = 680px of chrome / stacked overflow" case is closed by making the two side panels mutually exclusive — opening one closes the other. Verified at a 320px viewport by the new capture harness (screenshot in the PR body) and pinned in MeetingsTranslation.test.tsx.

  • rebutted — keep the Languages button when no language is configured.

    hiding the button makes the feature undiscoverable from the one place users would want it.

    This is the author's documented choice, kept deliberately: the code comment states the rationale ("with translation off the button would open a panel that can never fill. Settings is where it gets turned on."), and the control now lives in the overflow menu where a dead menu item would still be a dead end, not a discovery aid. Changing the discovery model is a product choice for the author/maintainer, not a drive fix.

@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Dispositions for the First Principles Review items on ad7b5685:

  • rebutted (proportionality) — delete TranslationQueue.drain().

    grepped .drain( across the diff: only test/test_meetings_translation.py calls it ...

    Accurate observation, but the demanded change removes a small deliberate test seam: drain() is what lets the 51-test suite flush the queue deterministically instead of awaiting private worker-task internals. Deleting public-for-tests surface and rewriting the tests around private state is a net reduction in test clarity for zero behavior change, and reshaping the author's API is outside this drive's mandate. The stale docstring claim ("Used at meeting teardown") is real; worth a one-line docstring correction if the author touches the file again.

  • rebutted (proportionality) — drop the enabled property and enqueue's empty-language guard.

    session.py.__post_init__ only constructs the queue when language in k.TRANSLATION_LANG_CODES ... so enabled is always True in production.

    True today, but the guard is the queue class defending its own invariant rather than trusting every current and future construction site — removing a cheap guard because the sole present caller happens to uphold it inverts the usual direction of review pressure. Keeping correct defensive code on a class boundary is not gold-plating; deleting it is churn with a regression surface.

  • accepted-and-deferred — 4th copy of the get_or_create → REJECT_ALL → release+destroy one-shot skeleton.

    A shared helper is the general fix — larger than this change and an addition, so accepted-and-deferred.

    Concur with the reviewer's own disposition: a shared one-shot helper spans four call sites in three modules and is a refactor with its own review surface, not something to smuggle into a feature PR. The reviewer's note that run_bg_oneliner's single _bg session would serialize translation against title-gen is the design constraint any such helper must honor — recorded here for whoever picks it up.

  • noted — the ja/ko transcript block re-sort was undeclared in the body at the reviewed head. The new head (197b00e3ba36) extends that normalization to all 13 catalogs deliberately (matching the sortDeep form the i18n tooling writes, so future automated runs carry no reorder churn) and the PR body now declares it.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Aug 29, 2026
@NicholasRBowers
NicholasRBowers force-pushed the pr/meetings-transcript-translation branch from 197b00e to 5bb7c27 Compare August 29, 2026 06:15
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 29, 2026
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Dispositions for the GPT 5.6 Review findings on 197b00e3:

  • fixedtranslate.py:224 cancelled persistence can recreate a deleted meeting.

    End during persistence -> worker cancellation -> delete returns 204 -> uncancelled thread recreates the meeting directory.

    Applied the prescribed fix in 5bb7c27f8eb2: append_translation now checks the meeting's metadata inside its existing meta_transaction and returns None without writing when it is gone — delete_meeting takes the same transaction around its rmtree, so the guard cannot interleave with the deletion. Red-before-verified regression test (test_append_refuses_to_recreate_a_deleted_meeting) pins that the entry is refused and the directory stays deleted. A focused verifier traced the race closed with no new defects; its one note — the skip is logged nowhere — is recorded here as accepted (behaviorally correct, the dropped lines belong to a meeting the user just deleted).

  • fixeduseMeetingSession.ts:373 language reset permanently skips initial translations.

    Restart with a new language -> old cursor queries reset numbering -> initial new-language lines fall below since and never render.

    Applied the prescribed fix in 5bb7c27f8eb2: on observing a server-language change the hook clears the accumulator, resets the cursor to zero, and refetches from zero before merging/advancing. The reset is keyed on the last observed server language (new lastServerLanguageRef) rather than live config — comparing against config would wipe the map on every poll for as long as a running session and a changed Settings value disagree. Test pin updated to the new shape (cursor reset + refetch-from-zero asserted between the language check and the merge loop), red-before verified.

  • The advisory FINDING (session.py:441translation_language captured at session creation) is the config-vs-live-language seam already dispositioned on this PR as a documented limitation per the feature's declared contract ("the target language is a per-meeting setting"); see the Design/UX disposition comments above.

Scope note from the focused verification, for the record: append_transcript (store.py) has an analogous mkdir-under-a-different-lock window — pre-existing on main, untouched by this PR's diff, and therefore not addressed here.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 29, 2026
@NicholasRBowers
NicholasRBowers force-pushed the pr/meetings-transcript-translation branch from 5bb7c27 to 106b432 Compare August 29, 2026 09:24
@NicholasRBowers

Copy link
Copy Markdown
Contributor

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Disposition for the Opus 4.8 Review advisory finding on 106b43236:

  • accepted-and-deferred[chat] prefix leaks into the translation panel's source column and the translation prompt for typed lines.

    handle_dispatch_text (agents.py:344) builds line = f"{k.CHAT_PREFIX} {transcript_text}" ... the added enqueue in broadcast (session.py:483) passes that prefixed text straight to self.translations.enqueue(...) ... TranslationSidebar renders {line.source} verbatim.

    Confirmed against the code: the reviewer's chain is accurate — the transcript path stores clean transcript_text while the translation path receives the prefixed broadcast line, so typed lines show the literal [chat] marker in the panel and carry it into the prompt. It is a real but minor cosmetic defect on a secondary path (typed chat lines, not transcription), and the reviewer graded it advisory, not blocking. This PR's head is otherwise fully green across all 70 checks; folding a cosmetic strip into it now would re-arm every CI and review lane for a one-line change. Deferred to a concrete follow-up task: fix(meetings): chat prefix leaks into translation source and prompt for typed lines #6763 (strip CHAT_PREFIX before enqueue + regression test asserting a clean source for [chat]-prefixed dispatches).

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 29, 2026
@bolichen97
bolichen97 enabled auto-merge August 30, 2026 00:01
bolichen97
bolichen97 previously approved these changes Aug 30, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved after a full-diff review (scope match, no out-of-scope files, security surface checked, tests verified non-vacuous). Review notes available on request.

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Aug 30, 2026
A meeting whose participants do not share a language is hard to read
afterwards. Each transcript line can now be translated on demand,
cached per meeting so a line is translated once, and shown beside the
original in a sidebar instead of replacing it.

The target language is a per-meeting setting, and translation runs
through the agent the meeting already uses.

Original feature by Kai Mitsuzawa (kaizawa97). Review fixes by Kiro
Crew: resolved the additive i18n locale conflicts from the rebase onto
main, released the translation sidebar's fixed 340px width below `lg`
(stacked with a bounded height, the shape TaskSidebar already uses so a
320px viewport no longer clips it), and moved the meeting toolbar's
secondary actions (end-and-review, refresh, translation, action items)
into an overflow menu so the row stays within the two-control cap.
Second review round: append_translation now refuses to write when the
meeting's metadata is gone, so a worker persistence racing
delete_meeting can no longer recreate the deleted directory; and the
client resets its cursor (and refetches from zero) when the server's
document language changes, so a restarted document's initial lines are
never skipped and the accumulator is keyed on the observed server
language rather than live config.

Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
@bolichen97
bolichen97 force-pushed the pr/meetings-transcript-translation branch from 106b432 to 133a2fd Compare August 30, 2026 04:35
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 30, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving after conflict-resolution rebase (single dataclass conflict in meetings session.py: main's init_buffer/init_dropped fields and this PR's root/translations fields both kept; AST-verified; single commit, scope unchanged).

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge labels Aug 30, 2026
@bolichen97
bolichen97 merged commit 4357be2 into kirodotdev:main Aug 30, 2026
52 of 63 checks passed
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Aug 30, 2026
@chenmingwei23 chenmingwei23 removed the drive-to-green PR claimed by drive-to-green pipeline label Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants