Skip to content

Add brainstorming session: TalkTerm concept review and revision - #2

Merged
don-petry merged 3 commits into
mainfrom
initialize-prd
Mar 21, 2026
Merged

don-petry merged 3 commits into
mainfrom
initialize-prd

Conversation

@don-petry

Copy link
Copy Markdown
Contributor

Introduces the TalkTerm concept brainstorming session notes and initial PRD work.

Progressive brainstorming session that revised the product vision from
"friendly terminal" to "voice-enabled avatar agent" for non-technical users.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 21, 2026 04:55
Complete PRD covering executive summary, success criteria, user journeys,
innovation analysis, mobile app requirements, phased scope, 35 functional
requirements, and 15 non-functional requirements.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI 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.

Pull request overview

Adds a new BMAD brainstorming session artifact capturing the “TalkTerm full concept review and revision” notes, intended to document the concept pivot and early PRD-direction decisions in the _bmad-output area.

Changes:

  • Adds a new brainstorming session markdown file with YAML frontmatter state tracking.
  • Documents key insights, revised positioning, MVP decisions, and deferred technical choices for TalkTerm.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread _bmad-output/brainstorming/brainstorming-session-2026-03-20.md
Comment thread _bmad-output/brainstorming/brainstorming-session-2026-03-20.md
Comment thread _bmad-output/brainstorming/brainstorming-session-2026-03-20.md
Comment thread _bmad-output/brainstorming/brainstorming-session-2026-03-20.md
Full review cycle: validation (12-step BMAD), adversarial review (15
findings), edge case hunt (20 unhandled paths), pre-mortem analysis.

Key improvements applied:
- Fix 15/20-min success criteria contradiction (→ ≤25 min)
- Decompose NFR1 latency into acknowledgement (≤1s) + first-response
  (≤3s) via streaming TTS; retire implausible 2s end-to-end target
- Add FR16 (file upload), FR36 (user identity), FR37 (barge-in),
  FR38 (network recovery)
- Remove implementation leakage: "Claude API" → "AI agent backend"
  in FR11/NFR13; remove NFR14 (architecture constraint)
- Elevate store compliance from deferred to required pre-design gate
- Add BYOK API key model with rationale
- Quantify all business success metrics
- Harden 10 FR + 8 NFR measurability specs
- Add NFR16 (conversation data privacy)
- Annotate Journey 2 Phase 2 persona references
- Structural: remove redundant Project Classification section,
  merge Executive Summary sub-heading, condense Phase 3 list
- 10 prose editorial fixes

Add prd-validation-report.md with full 12-step validation findings.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@don-petry
don-petry merged commit 77cbedd into main Mar 21, 2026
2 checks passed
don-petry added a commit that referenced this pull request Oct 11, 2026
…to-review

- Use explicit .length == 1 check instead of !pull_requests[1] to robustly detect multiple PRs (fixes issue #1)
- Add head_commit.id == github.sha check to prevent delayed old events from cancelling newer runs (fixes issue #2)
- Both fixes apply to check_suite and workflow_run event conditions

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
don-petry added a commit that referenced this pull request Oct 11, 2026
…eview.yml-concurrency (#521)

* feat: implement issue #508 — Compliance: stub-surface-drift-pr-auto-review.yml-concurrency

* chore: dev-lead update (review-changes) [skip ci-relay]

* fix(bot): improve docstring coverage in test-pr-auto-review-workflow.test.sh

Add single-line docstrings to 11 shell functions to improve docstring
coverage from 63.64% to above 80% threshold, as required by CodeRabbit's
docstring coverage check.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(bot): improve docstring coverage and validation logic in workflow guard scripts

- Add docstrings to 11 test helper functions (63.64% → 80% coverage)
- Improve concurrency.group validation checks (3, 4, 5) to verify complete,
  properly guarded expression branches using grep pattern matching, rather than
  accepting required references found in unrelated text
- Use grep to check for check_suite/workflow_run patterns with both PR number
  and head_sha in the same event block, ensuring commit-scoped isolation
- All guard checks and test fixtures continue to pass

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(reviews): validate full cancel-in-progress expression in pr-auto-review guard

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(bot): improve multi-PR detection and race condition prevention in pr-auto-review

- Changed array existence check from `!pull_requests[1]` to explicit `(pull_requests[1] == null)` for more reliable multi-PR detection
- Fixed cancel-in-progress to only cancel when using head_sha-based groups (single PR case), preventing delayed completion events from canceling unrelated runs
- These fixes prevent runs from different PRs sharing the same cancellable group when a commit belongs to multiple PRs

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(reviews): accept single-PR cancel-in-progress expression in pr-auto-review guard

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(bot): add explicit null-safety checks in pr-auto-review concurrency expressions

- Added explicit checks for pull_requests[0] existence before accessing .number property
- This prevents potential issues when accessing array elements that may not exist
- Updated validation script to accept the more defensive null-safety form
- These changes address CodeAnt's concerns about multi-PR detection logic

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(bot): restore pr-auto-review concurrency block to match canonical template

Replace custom head_sha and multi-PR logic with the canonical template's
simpler PR-number-based grouping and event-conditional cancellation per issue #508.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(reviews): align pr-auto-review guard with canonical PR-number-only concurrency

The stub is synced verbatim from the org template (PR-number-keyed group),
so the guard now accepts it instead of requiring head_sha / pull_requests[1].
Checks 3-4 are scoped per event branch.

Test-Change-Justification: the canonical stub restored in ff1f0ef is PR-number-only; the old Case 3/4 rejected exactly that policy, which made CI fail (review threads PRRT_kwDORsnIRM6rJpgj, PRRT_kwDORsnIRM6rKUi4 on PR #521, #521)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): require PR number in format() args of each concurrency branch

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): require PR number as {0} arg and run_id as terminal fallback in concurrency guard

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): reject run-unique values in per-PR concurrency branches

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): harden pr-auto-review concurrency guard

Require PR-number gating per branch, shared group format, ${{ }} delimiters,
and reject event-specific ids in per-PR keys.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): tighten pr-auto-review concurrency guard checks

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(reviews): reject escaped fallback placeholder and unwrapped cancel-in-progress in guard

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(bot): add multi-PR guards to pr-auto-review concurrency conditions

Add explicit checks to reject multi-PR events from the per-PR concurrency
groups, ensuring only single-PR events use PR-number deduplication while
multi-PR and no-PR events fall back to unique run_id groups.

Fixes: codeant-ai concern about concurrent events with multiple PRs
       potentially suppressing review dispatch for unrelated PRs.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(bot): resolve concurrency guard logic and race condition in pr-auto-review

- Use explicit .length == 1 check instead of !pull_requests[1] to robustly detect multiple PRs (fixes issue #1)
- Add head_commit.id == github.sha check to prevent delayed old events from cancelling newer runs (fixes issue #2)
- Both fixes apply to check_suite and workflow_run event conditions

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(reviews): gate pr-auto-review concurrency on pull_requests[0].number

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* fix(compliance): restore canonical pr-auto-review concurrency block

Issue #508 asks for the concurrency surface to be re-synced verbatim from
petry-projects/.github standards/workflows/pr-auto-review.yml. Earlier review
passes added a pull_requests[1] guard to the check_suite and workflow_run
branches of the group expression, which the compliance audit's
stub_surface_drift comparison still reports as drift. Drop the two guards so
the block matches the canonical template; a multi-PR guard, if wanted, belongs
in the template.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9

* test(compliance): key the event-name mutation on the canonical group text

The self-test's "check_suite branch not gated on its event name" mutation
matched the pull_requests[1] guard that the previous commit removed, so the
sed no longer applied and the case failed with "mutation did not apply".
Match the canonical line instead; the guard still rejects the mutant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9

* fix(bot): add multi-PR guard to prevent cross-PR group collisions

Strengthen the concurrency control by adding !pull_requests[1] checks to
ensure that events affecting multiple PRs fall back to the run-unique group
instead of mixing work across PRs. Update the regression guard to enforce
this constraint and add a test case verifying rejection of configurations
without the multi-PR guard.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(compliance): restore canonical pr-auto-review concurrency block (again)

Reverts ec9ff01, which re-added the pull_requests[1] guard to the group
expression after a reviewer-bot comment. Issue #508 requires the concurrency
surface to match petry-projects/.github standards/workflows/pr-auto-review.yml
verbatim; with the guard the compliance audit's stub_surface_drift comparison
still reports drift. The tree is identical to 28d9085, on which all required
checks and the workflow regression guards passed.

The maintainer has chosen to merge this PR by hand; it carries
dev-lead:hands-off so the guard is not added back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U1bsmUpeti2XUHnhQxWHU9

---------

Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
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.

2 participants