feat: add discovery sessions (DS-xxx) - #77
Merged
Merged
Conversation
…citation Introduce a new "discovery" artifact type with a 9-tool lifecycle for conducting stakeholder elicitation sessions, recording findings and gaps, reviewing outcomes, and iterating via follow-ups. Tools: start_discovery, record_finding, record_gap, complete_discovery, list_discoveries, get_discovery, add_discovery_review, resolve_gap, request_followup. Also adds onboarding step, health check, persona/prompt integration, and plugin prompt fragments for both generic-agile and sap-aem.
…sions Add discovery to persona document type lists and include the new DS-xxx type in the document types reference table with its statuses and fields.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant PO as Product Owner
participant Tools as Discovery Tools
participant Store as DocumentStore
participant Plugins as Plugins
PO->>Tools: start_discovery()
Tools->>Store: create Discovery (DS-1, draft)
Tools->>Store: inject parent gaps if chained
PO->>Tools: record_finding(session_id, finding)
Tools->>Store: append F-N block
PO->>Tools: record_gap(session_id, gap)
Tools->>Store: append GAP-N block (open)
Tools->>Store: optionally spawn Question artifact
PO->>Tools: complete_discovery(session_id)
Tools->>Store: transition to in-review
Tools->>Store: append summary (finding/gap counts)
PO->>Tools: review_discovery(session_id, annotation)
Tools->>Store: append review annotation
PO->>Tools: resolve_gap(session_id, gap_id, rationale)
Tools->>Store: update GAP-N to resolved
Tools->>Store: mark spawned Question as answered
PO->>Tools: request_follow_up(session_id)
Tools->>Store: transition to needs-input
Tools->>Store: append follow-up section
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/doctor/health/onboarding.ts (1)
86-134:⚠️ Potential issue | 🟡 MinorStep-number comments are now stale.
With the new discovery step inserted as Step 3, the subsequent inline comments no longer match: Line 96 says "Step 4: Break down into epics" (now 5), Line 107 says "Step 5: Set up Sprint 0" (now 6), Line 117 says "Step 6: Configure Jira integration" (now 7), and Line 126 says "Step 7: Run a health check" (now 8). Line 86 was correctly updated to "Step 4". Functionality is unaffected (orders come from
order++), but the comments are misleading for future readers.✏️ Proposed fix
- // Step 4: Break down into epics + // Step 5: Break down into epics steps.push({ order: order++, title: "Break work into epics", @@ - // Step 5: Set up Sprint 0 + // Step 6: Set up Sprint 0 steps.push({ order: order++, title: "Set up Sprint 0", @@ - // Step 6: Configure Jira integration + // Step 7: Configure Jira integration steps.push({ order: order++, title: "Configure Jira integration", @@ - // Step 7: Run a health check + // Step 8: Run a health check steps.push({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/doctor/health/onboarding.ts` around lines 86 - 134, The inline step-number comments are stale after inserting the discovery step; update the comment text in the subsequent steps so they match their actual sequence generated by order++ (e.g., change "Step 4: Break down into epics" etc. to the correct numbers); find the steps created via the repeated steps.push({ ... }) blocks (look for the objects with title "Break work into epics", "Set up Sprint 0", "Configure Jira integration", and "Run a health check") and edit their leading comment lines to reflect the new step numbers while leaving the order++ logic and fields (tool, done, description) unchanged.
🧹 Nitpick comments (1)
src/plugins/builtin/tools/discoveries.ts (1)
57-83: Refactor duplicated gap-block parser.The line-by-line GAP block scanner is repeated almost identically in
start_discovery(parent carry-forward) andrequest_followup(unresolved items). Extracting a small helper such ascollectOpenGapBlocks(content: string): { heading: string; block: string }[]would remove duplication, make the two call sites declarative, and centralize any future fix (e.g., handling####subheadings or resolved-in-child gaps).Also applies to: 479-506
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/builtin/tools/discoveries.ts` around lines 57 - 83, The GAP-block parsing logic is duplicated in start_discovery and request_followup; extract it into a small helper (suggested name collectOpenGapBlocks(content: string): { heading: string; block: string }[]) that takes document content and returns an array of open gap objects (heading and full block text). Replace the inlined scanner in both start_discovery and request_followup with calls to this helper, and ensure the helper preserves the existing behavior (detecting "### GAP-<n>:" headings, collecting until the next "### " heading, and only returning blocks containing "**Status:** open"); centralize any future parsing adjustments (e.g., handling deeper subheadings or resolved-in-child logic) in this new function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/builtin/tools/discoveries.ts`:
- Around line 248-271: The current completion path appends a new "## Session
Summary" to doc.content every time, causing duplicate summaries when
transitioning from needs-input to in-review again; update the logic in the
completion handler (the block that reads doc.frontmatter.status, computes
findingCount/gapCount, builds summary, and calls store.update) to detect an
existing "## Session Summary" section in doc.content and either replace that
section with the new summary or skip appending if the existing summary is
already up-to-date; use the existing utilities (e.g., countBlocks) to compute
counts, locate the existing summary by searching for the header line "## Session
Summary" and the section that follows it, and then call store.update(args.id, {
status: "in-review" }, newContent) with the modified content instead of always
appending.
- Around line 408-426: The handler currently only checks for gapHeading but then
unconditionally runs the replace and store.update; detect whether the GAP is
already resolved by testing doc.content for the open-status pattern before
performing the replace: use the same regex (or a RegExp.test) that matches `###
GAP-${args.gap_number}:[^]*?\\*\\*Status:\\*\\* open` to determine if the gap is
open, and only perform the replace and call store.update(args.id, {},
newContent) when that test passes; if the test fails (status not open) return an
explicit already-resolved response (or an error) instead of claiming a
successful resolution.
In `@test/plugins/tools/discoveries.test.ts`:
- Around line 88-91: The inline comment "Resolve one gap" is misleading because
the test calls complete_discovery (function complete_discovery) which only
transitions discovery status and appends a summary, it does not resolve gaps;
update or remove that comment in the test
(test/plugins/tools/discoveries.test.ts) so it accurately describes the action
(e.g., "Mark discovery complete / append summary") or simply delete the line to
avoid confusion with the following assertions that gaps remain open.
---
Outside diff comments:
In `@src/doctor/health/onboarding.ts`:
- Around line 86-134: The inline step-number comments are stale after inserting
the discovery step; update the comment text in the subsequent steps so they
match their actual sequence generated by order++ (e.g., change "Step 4: Break
down into epics" etc. to the correct numbers); find the steps created via the
repeated steps.push({ ... }) blocks (look for the objects with title "Break work
into epics", "Set up Sprint 0", "Configure Jira integration", and "Run a health
check") and edit their leading comment lines to reflect the new step numbers
while leaving the order++ logic and fields (tool, done, description) unchanged.
---
Nitpick comments:
In `@src/plugins/builtin/tools/discoveries.ts`:
- Around line 57-83: The GAP-block parsing logic is duplicated in
start_discovery and request_followup; extract it into a small helper (suggested
name collectOpenGapBlocks(content: string): { heading: string; block: string
}[]) that takes document content and returns an array of open gap objects
(heading and full block text). Replace the inlined scanner in both
start_discovery and request_followup with calls to this helper, and ensure the
helper preserves the existing behavior (detecting "### GAP-<n>:" headings,
collecting until the next "### " heading, and only returning blocks containing
"**Status:** open"); centralize any future parsing adjustments (e.g., handling
deeper subheadings or resolved-in-child logic) in this new function.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9202e768-8b9f-429c-95fa-5a2148267032
📒 Files selected for processing (18)
docs/guides/personas.mddocs/reference/document-types.mdsrc/core/statuses.tssrc/doctor/health/checks/index.tssrc/doctor/health/checks/no-discoveries.tssrc/doctor/health/onboarding.tssrc/personas/builtin/delivery-manager.tssrc/personas/builtin/product-owner.tssrc/personas/builtin/tech-lead.tssrc/personas/prompt-builder.tssrc/plugins/builtin/generic-agile.tssrc/plugins/builtin/sap-aem.tssrc/plugins/builtin/tools/discoveries.tssrc/plugins/common.tstest/doctor/health/engine.test.tstest/doctor/health/onboarding.test.tstest/plugins/registry.test.tstest/plugins/tools/discoveries.test.ts
- Extract shared gap-parsing logic into collectOpenGaps() helper, replacing duplicated scanners in start_discovery and request_followup - Replace existing "## Session Summary" on re-completion instead of appending a duplicate (needs-input -> in-review path) - Return explicit error when resolve_gap targets an already-resolved gap - Fix misleading test comment about complete_discovery - Renumber stale step comments in onboarding.ts (steps 5-8) - Add test for duplicate summary prevention and already-resolved gap
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
start_discovery,record_finding,record_gap,complete_discovery,list_discoveries,get_discovery,add_discovery_review,resolve_gap,request_followupno-discoveriesrecommendation), persona document types, and prompt fragments for both generic-agile and sap-aem pluginsKey design decisions
parent— open gaps carry forward automatically andsessionincrementsrecord_gapwithspawn_question: truecreates a linked Q-xxx withdiscovery:DS-xxxtag;resolve_gapauto-updates the spawned question toansweredTest plan
test/plugins/tools/discoveries.test.tscovering all 9 toolstest/doctor/health/engine.test.tsnpm run typecheck— passesnpm test— 875/875 passingnpm run lint— 0 errorsnpm run build— clean