Skip to content

Commit f8f8b97

Browse files
sylvansysclaude
andcommitted
fix: Improve agent prompt clarity for JSON output
- Use concrete examples instead of "passed|failed|skipped" placeholders - Add clear instruction to replace example values with actual findings - Remove debug logging from requirements-reviewer The placeholder format was causing Claude to output the literal text instead of choosing one status value. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f59278c commit f8f8b97

3 files changed

Lines changed: 45 additions & 67 deletions

File tree

.constellos/agents/code-quality.md

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -46,61 +46,58 @@ You must evaluate each of these checks independently:
4646

4747
## Output Format
4848

49-
**CRITICAL**: Output ONLY the JSON block below. Each check MUST have a status.
49+
**CRITICAL**: After your review, output a single JSON block with your findings.
5050

51+
For each check, set `status` to exactly one of: `"passed"`, `"failed"`, or `"skipped"`.
52+
- Use `"passed"` if no issues found
53+
- Use `"failed"` if any issues found
54+
- Use `"skipped"` if check is not applicable
55+
56+
Example output:
5157
```json
5258
{
5359
"checks": [
5460
{
5561
"name": "DRY",
56-
"status": "passed|failed|skipped",
57-
"result": "Brief 1-line result",
58-
"reasoning": "Why this status was given",
59-
"files": [
60-
{
61-
"path": "path/to/file.ts",
62-
"line": 42,
63-
"note": "Specific issue or observation"
64-
}
65-
]
62+
"status": "passed",
63+
"result": "No code duplication found",
64+
"reasoning": "Each component has unique logic with no repeated patterns",
65+
"files": []
6666
},
6767
{
6868
"name": "YAGNI",
69-
"status": "passed|failed|skipped",
70-
"result": "Brief 1-line result",
71-
"reasoning": "Why this status was given",
69+
"status": "passed",
70+
"result": "No over-engineering detected",
71+
"reasoning": "Implementation matches requirements without unnecessary complexity",
7272
"files": []
7373
},
7474
{
7575
"name": "Modularity",
76-
"status": "passed|failed|skipped",
77-
"result": "Brief 1-line result",
78-
"reasoning": "Why this status was given",
76+
"status": "passed",
77+
"result": "Good separation of concerns",
78+
"reasoning": "Components have single responsibilities and clear interfaces",
7979
"files": []
8080
},
8181
{
8282
"name": "Maintainability",
83-
"status": "passed|failed|skipped",
84-
"result": "Brief 1-line result",
85-
"reasoning": "Why this status was given",
83+
"status": "passed",
84+
"result": "Code is readable and well-structured",
85+
"reasoning": "Clear naming, appropriate comments, logical organization",
8686
"files": []
8787
},
8888
{
8989
"name": "Error Handling",
90-
"status": "passed|failed|skipped",
91-
"result": "Brief 1-line result",
92-
"reasoning": "Why this status was given",
90+
"status": "skipped",
91+
"result": "No error handling code in changes",
92+
"reasoning": "Changes are configuration files only",
9393
"files": []
9494
}
9595
],
96-
"message": "Optional overall assessment (leave empty if not needed)"
96+
"message": ""
9797
}
9898
```
9999

100-
## Status Guidelines
101-
- **passed**: No issues found for this check
102-
- **failed**: One or more issues found - be strict, fail if there are ANY violations
103-
- **skipped**: Check not applicable (e.g., no error handling code to review)
100+
**Important**: Replace the example values above with your actual findings. The status must be one of: `passed`, `failed`, or `skipped`.
104101

105102
## Severity in Files
106103
When adding files, prioritize high-severity issues:

.constellos/agents/requirements.md

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,47 +38,43 @@ You must evaluate each of these checks independently:
3838

3939
## Output Format
4040

41-
**CRITICAL**: Output ONLY the JSON block below. Each check MUST have a status.
41+
**CRITICAL**: After your review, output a single JSON block with your findings.
42+
43+
For each check, set `status` to exactly one of: `"passed"`, `"failed"`, or `"skipped"`.
44+
- Use `"passed"` if check criteria are fully satisfied
45+
- Use `"failed"` if any issues are found
46+
- Use `"skipped"` if check is not applicable (e.g., no issue context)
4247

4348
```json
4449
{
4550
"checks": [
4651
{
4752
"name": "Completeness",
48-
"status": "passed|failed|skipped",
49-
"result": "Brief 1-line result",
50-
"reasoning": "Why this status was given",
51-
"files": [
52-
{
53-
"path": "path/to/file.ts",
54-
"line": 42,
55-
"note": "Missing requirement X implementation"
56-
}
57-
]
53+
"status": "passed",
54+
"result": "All acceptance criteria implemented",
55+
"reasoning": "Each requirement from the issue is addressed in the code changes",
56+
"files": []
5857
},
5958
{
6059
"name": "Scope",
61-
"status": "passed|failed|skipped",
62-
"result": "Brief 1-line result",
63-
"reasoning": "Why this status was given",
60+
"status": "passed",
61+
"result": "Changes within scope",
62+
"reasoning": "All changes relate directly to the issue requirements",
6463
"files": []
6564
},
6665
{
6766
"name": "Traceability",
68-
"status": "passed|failed|skipped",
69-
"result": "Brief 1-line result",
70-
"reasoning": "Why this status was given",
67+
"status": "skipped",
68+
"result": "No tests required for config change",
69+
"reasoning": "This is a workflow configuration change, not a code change requiring tests",
7170
"files": []
7271
}
7372
],
74-
"message": "Optional overall assessment (leave empty if not needed)"
73+
"message": ""
7574
}
7675
```
7776

78-
## Status Guidelines
79-
- **passed**: Check criteria fully satisfied
80-
- **failed**: One or more issues found - be strict, fail if there are ANY violations
81-
- **skipped**: Check not applicable (e.g., no issue context available)
77+
**Important**: Replace the example values above with your actual findings. The status must be one of: `passed`, `failed`, or `skipped`.
8278

8379
## Guidelines
8480
- Be strict about scope - extra features should be flagged

.github/actions/requirements-reviewer/action.yml

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,10 @@ runs:
5252
5353
BRANCH="${{ inputs.branch }}"
5454
echo "branch=$BRANCH" >> $GITHUB_OUTPUT
55-
echo "DEBUG: Branch name is '$BRANCH'"
5655
5756
# Extract issue number from branch name (e.g., "123-feature-name")
5857
ISSUE=""
59-
if [[ $BRANCH =~ ^([0-9]+)- ]]; then
60-
ISSUE="${BASH_REMATCH[1]}"
61-
echo "DEBUG: Extracted issue '$ISSUE' from branch name"
62-
else
63-
echo "DEBUG: Branch name did not match pattern ^([0-9]+)-"
64-
fi
58+
[[ $BRANCH =~ ^([0-9]+)- ]] && ISSUE="${BASH_REMATCH[1]}"
6559
6660
# Fallback 1: Parse PR body for "Closes #X", "Fixes #X", "Resolves #X"
6761
if [ -z "$ISSUE" ]; then
@@ -89,24 +83,15 @@ runs:
8983
[ "$ISSUE" = "null" ] && ISSUE=""
9084
fi
9185
92-
echo "DEBUG: Final issue number is '$ISSUE'"
9386
echo "issue=$ISSUE" >> $GITHUB_OUTPUT
9487
9588
# Get changed files (gracefully handle permission errors)
9689
gh pr view ${{ inputs.pr_number }} --json files --jq '.files[].path' > .claude/review-context/changed.txt 2>/dev/null || touch .claude/review-context/changed.txt
9790
9891
# Get issue context if available
9992
if [ -n "$ISSUE" ]; then
100-
echo "DEBUG: Fetching issue $ISSUE..."
101-
if gh issue view $ISSUE --json title,body > .claude/review-context/issue.json 2>&1; then
102-
echo "DEBUG: Issue fetch succeeded, content:"
103-
cat .claude/review-context/issue.json | head -c 200
104-
else
105-
echo "DEBUG: Issue fetch failed, using empty JSON"
106-
echo "{}" > .claude/review-context/issue.json
107-
fi
93+
gh issue view $ISSUE --json title,body > .claude/review-context/issue.json 2>/dev/null || echo "{}" > .claude/review-context/issue.json
10894
else
109-
echo "DEBUG: No issue number, using empty JSON"
11095
echo "{}" > .claude/review-context/issue.json
11196
fi
11297

0 commit comments

Comments
 (0)