docs: add local development instructions - #17
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a ChangesFeature research workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ResearchAgents
participant ScoringAgents
participant SkepticAgents
participant MeasurementAgents
participant Synthesizers
participant ScratchStorage
ResearchAgents->>ScoringAgents: aggregated research findings
ScoringAgents->>SkepticAgents: feature scores
ScoringAgents->>MeasurementAgents: scored feature candidates
SkepticAgents->>Synthesizers: skeptic decisions and objections
MeasurementAgents->>Synthesizers: measurement plans
Synthesizers->>ScratchStorage: Markdown report
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
🧹 Nitpick comments (3)
.grok/workflows/feature-research.rhai (3)
253-294: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused counters
fiandsi.
fi(Lines 255, 293) andsi(Lines 313, 357) are incremented but never read. The labels usefidinstead.♻️ Proposed cleanup
let sjobs = []; -let fi = 0; for row in features { @@ output_schema: score_schema, }); - fi += 1; }let kjobs = []; // Map scores by building parallel prompts aligned with feature catalog order -let si = 0; for row in features { @@ output_schema: skeptic_schema, }); - si += 1; }Also applies to: 311-358
🤖 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 @.grok/workflows/feature-research.rhai around lines 253 - 294, Remove the unused fi and si counter variables and their increment statements from the feature-scoring and subsequent job-building loops. Preserve the existing label construction using fid and all other job-generation behavior.
319-333: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the repeated linear lookups with maps keyed by
feature_id.The same scan-
scores-by-feature_idpattern appears four times, and the measure phase also scansskeptics. Build two maps once after the score and skeptic phases, then index them. This removes the duplication and the nested 40×40 scans.♻️ Proposed refactor
+let score_by_id = #{}; for r in score_results { if r != () && r.success && r.output != () && r.output.feature_id != () { s_ok += 1; scores.push(r.output); + score_by_id[r.output.feature_id] = r.output; } }Then replace each lookup loop with
let sc = score_by_id[fid];plus asc != ()guard. Apply the same pattern forskepticsvia askeptic_by_idmap.Also applies to: 381-404, 588-606
🤖 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 @.grok/workflows/feature-research.rhai around lines 319 - 333, Build score_by_id and skeptic_by_id maps once after the score and skeptic phases, keyed by feature_id. Replace each repeated scores scan, including the lookup near the score_summary construction and the other score lookup blocks, with indexed retrieval and a sc != () guard; update the measure-phase skeptics scan to use skeptic_by_id similarly. Preserve the existing no-match defaults and formatting behavior.
109-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the panel sizes from
feature_catalog()instead of restating them. The catalog length is hard-coded in a comment, in five log strings, intotal_agents, in the report Method section, and in the report coverage stats. The comment already disagrees with the catalog: it says 36 candidates and 125 agents, but the catalog holds 40 rows (F01–F40) and the real total is 137. Computelet feature_count = features.len();once and reuse it at every site.
.grok/workflows/feature-research.rhai#L109-L110: correct the comment to 40 candidates and 137 agents, or delete the count from the comment..grok/workflows/feature-research.rhai#L243-L243: replace the"/15"literal with the length ofresearch_angles..grok/workflows/feature-research.rhai#L305-L305: replace the"/40"literal withfeature_count..grok/workflows/feature-research.rhai#L369-L369: replace the"/40"literal withfeature_count..grok/workflows/feature-research.rhai#L440-L440: replace the"/40"literal withfeature_count..grok/workflows/feature-research.rhai#L496-L496: computetotal_agentsfromresearch_angles.len(),feature_count, and the synth job count..grok/workflows/feature-research.rhai#L536-L543: build the Method bullet counts from the same variables..grok/workflows/feature-research.rhai#L717-L728: build the coverage-stat denominators from the same variables.🤖 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 @.grok/workflows/feature-research.rhai around lines 109 - 110, Derive all feature-research panel sizes from feature_catalog() and shared variables instead of hard-coded counts. In .grok/workflows/feature-research.rhai#L109-L110, update or remove the stale comment; at `#L243` use research_angles.len(), at `#L305`, `#L369`, and `#L440` use feature_count; compute total_agents at `#L496` from research_angles.len(), feature_count, and synth jobs; and build Method counts at `#L536-L543` and coverage denominators at `#L717-L728` from those same variables. Define feature_count once from features.len() and reuse it throughout.
🤖 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 @.grok/workflows/feature-research.rhai:
- Around line 733-748: Update the primary assignment guard in the synth_results
handling to also require synth_results[0].output != () before assigning it to
primary, preserving the default empty map for successful results with no output
and ensuring complete() receives the intended synthesis value.
- Around line 109-110: Update the agent-count comment near the feature catalog
to reflect 40 candidates and the correct total calculation of 40 × 3 + 15 + 2 =
137, matching total_agents and the logs.
In `@README.md`:
- Around line 29-40: Remove the duplicate introductory Development section and
retain the existing ## Development section. Update the retained section to use
npm ci, include npm run verify:package, preserve the live DNS/npm installation
warning for npm run test:e2e, and add the accurate dist/ output detail and npm
run test:all command.
---
Nitpick comments:
In @.grok/workflows/feature-research.rhai:
- Around line 253-294: Remove the unused fi and si counter variables and their
increment statements from the feature-scoring and subsequent job-building loops.
Preserve the existing label construction using fid and all other job-generation
behavior.
- Around line 319-333: Build score_by_id and skeptic_by_id maps once after the
score and skeptic phases, keyed by feature_id. Replace each repeated scores
scan, including the lookup near the score_summary construction and the other
score lookup blocks, with indexed retrieval and a sc != () guard; update the
measure-phase skeptics scan to use skeptic_by_id similarly. Preserve the
existing no-match defaults and formatting behavior.
- Around line 109-110: Derive all feature-research panel sizes from
feature_catalog() and shared variables instead of hard-coded counts. In
.grok/workflows/feature-research.rhai#L109-L110, update or remove the stale
comment; at `#L243` use research_angles.len(), at `#L305`, `#L369`, and `#L440` use
feature_count; compute total_agents at `#L496` from research_angles.len(),
feature_count, and synth jobs; and build Method counts at `#L536-L543` and
coverage denominators at `#L717-L728` from those same variables. Define
feature_count once from features.len() and reuse it throughout.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b4a5007d-1791-4cbc-adc0-a026ab5dadc1
📒 Files selected for processing (2)
.grok/workflows/feature-research.rhaiREADME.md
| // Feature catalog: id|title|category|source|one_line | ||
| // 36 candidates × (score + skeptic + measure) + 15 research + 2 synth = 125 agents |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale agent-count comment.
The catalog contains 40 entries (F01–F40), not 36. The actual panel is 40 × 3 + 15 + 2 = 137, which matches total_agents on Line 496 and the logs. Update the comment so it does not contradict the code.
📝 Proposed fix
// Feature catalog: id|title|category|source|one_line
-// 36 candidates × (score + skeptic + measure) + 15 research + 2 synth = 125 agents
+// 40 candidates × (score + skeptic + measure) + 15 research + 2 synth = 137 agents
fn feature_catalog() {🤖 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 @.grok/workflows/feature-research.rhai around lines 109 - 110, Update the
agent-count comment near the feature catalog to reflect 40 candidates and the
correct total calculation of 40 × 3 + 15 + 2 = 137, matching total_agents and
the logs.
| // Prefer primary synth as complete value | ||
| let primary = #{}; | ||
| if synth_results.len() > 0 && synth_results[0] != () && synth_results[0].success { | ||
| primary = synth_results[0].output; | ||
| } | ||
|
|
||
| complete(#{ | ||
| path: path, | ||
| total_agents: total_agents, | ||
| research_ok: r_ok, | ||
| score_ok: s_ok, | ||
| skeptic_ok: k_ok, | ||
| measure_ok: m_ok, | ||
| synthesis: primary, | ||
| summary: "Evaluated 40 feature candidates with " + total_agents.to_string() + " logical agents; report at " + path, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Guard synth_results[0].output before assigning it to primary.
The condition checks success but not output. Every other result handler in this file also checks r.output != () (see Lines 300, 364, 435, 677). If the primary synthesizer reports success with an empty output, primary becomes () and the synthesis field in complete() is unit instead of the intended empty map. That contradicts the let primary = #{} default.
🐛 Proposed fix
let primary = #{};
-if synth_results.len() > 0 && synth_results[0] != () && synth_results[0].success {
+if synth_results.len() > 0 && synth_results[0] != () && synth_results[0].success && synth_results[0].output != () {
primary = synth_results[0].output;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Prefer primary synth as complete value | |
| let primary = #{}; | |
| if synth_results.len() > 0 && synth_results[0] != () && synth_results[0].success { | |
| primary = synth_results[0].output; | |
| } | |
| complete(#{ | |
| path: path, | |
| total_agents: total_agents, | |
| research_ok: r_ok, | |
| score_ok: s_ok, | |
| skeptic_ok: k_ok, | |
| measure_ok: m_ok, | |
| synthesis: primary, | |
| summary: "Evaluated 40 feature candidates with " + total_agents.to_string() + " logical agents; report at " + path, | |
| }); | |
| // Prefer primary synth as complete value | |
| let primary = #{}; | |
| if synth_results.len() > 0 && synth_results[0] != () && synth_results[0].success && synth_results[0].output != () { | |
| primary = synth_results[0].output; | |
| } | |
| complete(#{ | |
| path: path, | |
| total_agents: total_agents, | |
| research_ok: r_ok, | |
| score_ok: s_ok, | |
| skeptic_ok: k_ok, | |
| measure_ok: m_ok, | |
| synthesis: primary, | |
| summary: "Evaluated 40 feature candidates with " + total_agents.to_string() + " logical agents; report at " + path, | |
| }); |
🤖 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 @.grok/workflows/feature-research.rhai around lines 733 - 748, Update the
primary assignment guard in the synth_results handling to also require
synth_results[0].output != () before assigning it to primary, preserving the
default empty map for successful results with no output and ensuring complete()
receives the intended synthesis value.
| ## Development | ||
|
|
||
| Build the CLI locally and run the test suite: | ||
|
|
||
| ```bash | ||
| npm install | ||
| npm test | ||
| ``` | ||
|
|
||
| The compiled CLI is written to `dist/`. To exercise the end-to-end checks as | ||
| well, run `npm run test:all`. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove this section or merge it into the existing "## Development" section.
The README already contains a ## Development section at Lines 207-221. Two identical headings create two #development anchors, so any link to #development resolves only to the first one. The two sections also disagree:
- Line 34 uses
npm install; Line 214 usesnpm ci. - This section omits the caveat at Lines 220-221 that
npm run test:e2eperforms a live DNS lookup and a real npm installation. A reader who followsnpm run test:allhere gets that live behavior without the warning. - This section omits
npm run verify:package.
The dist/ output path and the test:all script are correct per package.json. Keep the single existing section and add the dist/ detail there.
📝 Proposed fix: drop the duplicate and extend the existing section
-## Development
-
-Build the CLI locally and run the test suite:
-
-```bash
-npm install
-npm test
-```
-
-The compiled CLI is written to `dist/`. To exercise the end-to-end checks as
-well, run `npm run test:all`.
-Then extend the existing section:
`npm test` uses deterministic, mocked DNS responses. The E2E command is
separate because it performs a live DNS lookup and a real npm installation.
+
+The compiled CLI is written to `dist/`. Run `npm run test:all` to run the unit
+and end-to-end suites together.#!/bin/bash
# Description: Find links to the `#development` anchor and confirm the duplicate heading.
set -uo pipefail
rg -n '^## Development' README.md
rg -rn '`#development`' --iglob '*.md' --iglob '*.yml' --iglob '*.json' . || 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 `@README.md` around lines 29 - 40, Remove the duplicate introductory
Development section and retain the existing ## Development section. Update the
retained section to use npm ci, include npm run verify:package, preserve the
live DNS/npm installation warning for npm run test:e2e, and add the accurate
dist/ output detail and npm run test:all command.
Summary
Validation
pm test (244 passed)
Summary by CodeRabbit
New Features
Documentation