Claude: SRD Creator v2#970
Open
vprashrex wants to merge 1 commit into
Open
Conversation
…y, and improve quality checklist
📝 WalkthroughWalkthroughThe PR rewrites the SRD creator skill guidance, matching references, and diagram rendering helper to enforce PRD-based SRD generation and output conventions. It also adds a new SRD for LLM judge correctness, covering flow, endpoints, schema changes, defaults, and limitations. ChangesSRD Creator Guidance
LLM Judge Correctness SRD
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
OpenAPI changes ⚪ No API surface changesNote This PR does not modify the API contract.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
vprashrex
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.claude/skills/srd-creator/SKILL.md (1)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify "TodoWrite" intent.
Line 32 says "Load the Quality Checklist into a
TodoWritelist". Is "TodoWrite" a specific tool/extension reference (e.g., a VS Code extension), or should this read generically as "todo list"? If it's meant literally, consider lowercase without code formatting; if it's a specific tool, a brief parenthetical would help.🤖 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 @.claude/skills/srd-creator/SKILL.md at line 32, Clarify the meaning of “TodoWrite” in the Quality Checklist guidance so it is not ambiguous. In the SKILL.md instruction that references the TodoWrite list, either make it clearly generic as a todo list or, if it is a specific tool/reference, add a brief parenthetical explanation; keep the wording consistent with the surrounding checklist instructions and use the TodoWrite term only in the intended sense.features/llm-judge-correctness/SRD.md (1)
266-268: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the explicit constant name for the Correctness score.
The SRD notes the score name is "owned in Kaapi code as constants" and mirrors
COSINE_SCORE_NAME. For implementation clarity, specify the constant name here (e.g.,CORRECTNESS_SCORE_NAME = "Correctness") so the SRD serves as the source of truth for both the string value and its symbol.🤖 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 `@features/llm-judge-correctness/SRD.md` around lines 266 - 268, Add the explicit constant symbol for the Correctness score in the SRD so it is unambiguous and mirrors COSINE_SCORE_NAME; update the section describing the built-in default judge prompt to name the constant (for example, CORRECTNESS_SCORE_NAME) and state that it owns the "Correctness" string value. Keep the SRD aligned with the existing constant-based pattern used for score names so implementation can reference the symbol directly.
🤖 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 @.claude/skills/srd-creator/SKILL.md:
- Around line 138-150: The workflow instructions conflict with the rules about
diagram generation, so clarify the intended behavior in the SRD-creator skill.
Update the relevant guidance around Workflow step 6 and the diagram-related
rules so they agree on whether the skill actually renders the Mermaid diagram
via render-diagram.sh or only leaves an author placeholder for manual pasting.
Make the behavior explicit using the existing step 6 wording and the diagram
rules section so authors know exactly what to do.
---
Nitpick comments:
In @.claude/skills/srd-creator/SKILL.md:
- Line 32: Clarify the meaning of “TodoWrite” in the Quality Checklist guidance
so it is not ambiguous. In the SKILL.md instruction that references the
TodoWrite list, either make it clearly generic as a todo list or, if it is a
specific tool/reference, add a brief parenthetical explanation; keep the wording
consistent with the surrounding checklist instructions and use the TodoWrite
term only in the intended sense.
In `@features/llm-judge-correctness/SRD.md`:
- Around line 266-268: Add the explicit constant symbol for the Correctness
score in the SRD so it is unambiguous and mirrors COSINE_SCORE_NAME; update the
section describing the built-in default judge prompt to name the constant (for
example, CORRECTNESS_SCORE_NAME) and state that it owns the "Correctness" string
value. Keep the SRD aligned with the existing constant-based pattern used for
score names so implementation can reference the symbol directly.
🪄 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: a821e06f-da04-4346-b7c4-21d2a838b73a
⛔ Files ignored due to path filters (1)
features/llm-judge-correctness/assets/flow-a.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.claude/skills/srd-creator/SKILL.md.claude/skills/srd-creator/reference/srd-guide.md.claude/skills/srd-creator/reference/srd-template.md.claude/skills/srd-creator/scripts/render-diagram.shfeatures/llm-judge-correctness/SRD.md
Comment on lines
+138
to
+150
| - **Diagrams over prose for flows, but the SRD only holds a placeholder.** Both the | ||
| execution flow and the data model want a diagram, not a long numbered paragraph. | ||
| The skill does not draw it; it leaves an author image placeholder note (Workflow | ||
| step 6) and keeps the surrounding prose to what a diagram can't carry (failure | ||
| isolation, idempotency, resolution rules). Tell the author what the diagram should | ||
| depict so they can draw it: | ||
| - **Execution flow at system level, not internals.** The diagram should show the | ||
| user and the real systems that talk to each other (e.g. `User`, `Kaapi Backend`, | ||
| `OpenAI`, `Langfuse`), each with arrows in and out. It should not turn internal | ||
| pipeline steps or in-process helpers into separate lanes (e.g. a "Judge" lane | ||
| that is really an OpenAI call), which misleads readers into thinking they're | ||
| separate services. | ||
| - **Data model** should mark reused vs new entities. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
Resolve contradiction: does the skill render the diagram or not?
Workflow step 6 (lines 59–72) instructs the skill to "Write a mermaid source for the execution flow, then run the skill's helper script" to render the PNG. But the Rules (lines 138–150) state "The skill does not draw it; it leaves an author image placeholder note."
These two sections contradict each other. Clarify which is correct: does the skill auto-render via render-diagram.sh, or does it only leave the placeholder band for a human author to paste later?
🤖 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 @.claude/skills/srd-creator/SKILL.md around lines 138 - 150, The workflow
instructions conflict with the rules about diagram generation, so clarify the
intended behavior in the SRD-creator skill. Update the relevant guidance around
Workflow step 6 and the diagram-related rules so they agree on whether the skill
actually renders the Mermaid diagram via render-diagram.sh or only leaves an
author placeholder for manual pasting. Make the behavior explicit using the
existing step 6 wording and the diagram rules section so authors know exactly
what to do.
Prajna1999
approved these changes
Jun 26, 2026
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
2 participants
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.
Issue
Closes #971
Summary
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes