CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-05-05 05:05:08 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: framework-agnostic grep and error examples in plan reviews

Browse Source 
- CEO review: replace --include="*.rb" grep with --exclude-dir pattern,
  replace find -newer Gemfile.lock with git log --since, genericize
  Error & Rescue Map example table
- Eng review: "JS or Rails test" → "corresponding test"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-03-17 22:15:13 -07:00
parent cadd5a7f90
commit 37b9badf4b
4 changed files with 26 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plan-ceo-review/SKILL.md
+12 -12
View File
@@ -212,8 +212,8 @@ Run the following commands:
git log --oneline -30 # Recent history
git diff <base> --stat # What's already changed
git stash list # Any stashed work
grep -r "TODO\|FIXME\|HACK\|XXX" --include="*.rb" --include="*.js" -l
find . -name "*.rb" -newer Gemfile.lock | head -20 # Recently touched files
grep -r "TODO\|FIXME\|HACK\|XXX" -l --exclude-dir=node_modules --exclude-dir=vendor --exclude-dir=.git | head -30
git log --since=30.days --name-only --format="" | sort | uniq -c | sort -rn | head -20 # Recently touched files
```
Then read CLAUDE.md, TODOS.md, and any existing architecture docs. When reading TODOS.md, specifically:
* Note any TODOs this plan touches, blocks, or unlocks
@@ -397,24 +397,24 @@ For every new method, service, or codepath that can fail, fill in this table:
```
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
-------------------------|-----------------------------|-----------------
ExampleService#call | API timeout | Faraday::TimeoutError
ExampleService.call | API timeout | TimeoutError
| API returns 429 | RateLimitError
| API returns malformed JSON | JSON::ParserError
| DB connection pool exhausted| ActiveRecord::ConnectionTimeoutError
| Record not found | ActiveRecord::RecordNotFound
| API returns malformed JSON | JSONParseError
| DB connection pool exhausted| ConnectionPoolExhausted
| Record not found | RecordNotFound
-------------------------|-----------------------------|-----------------
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
-----------------------------|-----------|------------------------|------------------
Faraday::TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
RateLimitError | Y | Backoff + retry | Nothing (transparent)
JSON::ParserError | N ← GAP | — | 500 error ← BAD
ConnectionTimeoutError | N ← GAP | — | 500 error ← BAD
ActiveRecord::RecordNotFound | Y | Return nil, log warning | "Not found" message
JSONParseError | N ← GAP | — | 500 error ← BAD
ConnectionPoolExhausted | N ← GAP | — | 500 error ← BAD
RecordNotFound | Y | Return nil, log warning | "Not found" message
```
Rules for this section:
* `rescue StandardError` is ALWAYS a smell. Name the specific exceptions.
* `rescue => e` with only `Rails.logger.error(e.message)` is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
* Catching all errors generically (`rescue StandardError`, `catch (Exception e)`, `except Exception`) is ALWAYS a smell. Name the specific exceptions.
* Logging only the error message is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
* Every rescued error must either: retry with backoff, degrade gracefully with a user-visible message, or re-raise with added context. "Swallow and continue" is almost never acceptable.
* For each GAP (unrescued error that should be rescued): specify the rescue action and what the user should see.
* For LLM/AI service calls specifically: what happens when the response is malformed? When it's empty? When it hallucinates invalid JSON? When the model returns a refusal? Each of these is a distinct failure mode.
plan-ceo-review/SKILL.md.tmpl
+12 -12
View File
@@ -91,8 +91,8 @@ Run the following commands:
git log --oneline -30 # Recent history
git diff <base> --stat # What's already changed
git stash list # Any stashed work
grep -r "TODO\|FIXME\|HACK\|XXX" --include="*.rb" --include="*.js" -l
find . -name "*.rb" -newer Gemfile.lock | head -20 # Recently touched files
grep -r "TODO\|FIXME\|HACK\|XXX" -l --exclude-dir=node_modules --exclude-dir=vendor --exclude-dir=.git | head -30
git log --since=30.days --name-only --format="" | sort | uniq -c | sort -rn | head -20 # Recently touched files
```
Then read CLAUDE.md, TODOS.md, and any existing architecture docs. When reading TODOS.md, specifically:
* Note any TODOs this plan touches, blocks, or unlocks
@@ -276,24 +276,24 @@ For every new method, service, or codepath that can fail, fill in this table:
```
METHOD/CODEPATH | WHAT CAN GO WRONG | EXCEPTION CLASS
-------------------------|-----------------------------|-----------------
ExampleService#call | API timeout | Faraday::TimeoutError
ExampleService.call | API timeout | TimeoutError
| API returns 429 | RateLimitError
| API returns malformed JSON | JSON::ParserError
| DB connection pool exhausted| ActiveRecord::ConnectionTimeoutError
| Record not found | ActiveRecord::RecordNotFound
| API returns malformed JSON | JSONParseError
| DB connection pool exhausted| ConnectionPoolExhausted
| Record not found | RecordNotFound
-------------------------|-----------------------------|-----------------
EXCEPTION CLASS | RESCUED? | RESCUE ACTION | USER SEES
-----------------------------|-----------|------------------------|------------------
Faraday::TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
TimeoutError | Y | Retry 2x, then raise | "Service temporarily unavailable"
RateLimitError | Y | Backoff + retry | Nothing (transparent)
JSON::ParserError | N ← GAP | — | 500 error ← BAD
ConnectionTimeoutError | N ← GAP | — | 500 error ← BAD
ActiveRecord::RecordNotFound | Y | Return nil, log warning | "Not found" message
JSONParseError | N ← GAP | — | 500 error ← BAD
ConnectionPoolExhausted | N ← GAP | — | 500 error ← BAD
RecordNotFound | Y | Return nil, log warning | "Not found" message
```
Rules for this section:
* `rescue StandardError` is ALWAYS a smell. Name the specific exceptions.
* `rescue => e` with only `Rails.logger.error(e.message)` is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
* Catching all errors generically (`rescue StandardError`, `catch (Exception e)`, `except Exception`) is ALWAYS a smell. Name the specific exceptions.
* Logging only the error message is insufficient. Log the full context: what was being attempted, with what arguments, for what user/request.
* Every rescued error must either: retry with backoff, degrade gracefully with a user-visible message, or re-raise with added context. "Swallow and continue" is almost never acceptable.
* For each GAP (unrescued error that should be rescued): specify the rescue action and what the user should see.
* For LLM/AI service calls specifically: what happens when the response is malformed? When it's empty? When it hallucinates invalid JSON? When the model returns a refusal? Each of these is a distinct failure mode.
plan-eng-review/SKILL.md
+1 -1
View File
@@ -205,7 +205,7 @@ Evaluate:
**STOP.** For each issue found in this section, call AskUserQuestion individually. One issue per call. Present options, state your recommendation, explain WHY. Do NOT batch multiple issues into one AskUserQuestion. Only proceed to the next section after ALL issues in this section are resolved.
### 3. Test review
Make a diagram of all new UX, new data flow, new codepaths, and new branching if statements or outcomes. For each, note what is new about the features discussed in this branch and plan. Then, for each new item in the diagram, make sure there is a JS or Rails test.
Make a diagram of all new UX, new data flow, new codepaths, and new branching if statements or outcomes. For each, note what is new about the features discussed in this branch and plan. Then, for each new item in the diagram, make sure there is a corresponding test.
For LLM/prompt changes: check the "Prompt/LLM changes" file patterns listed in CLAUDE.md. If this plan touches ANY of those patterns, state which eval suites must be run, which cases should be added, and what baselines to compare against. Then use AskUserQuestion to confirm the eval scope with the user.
plan-eng-review/SKILL.md.tmpl
+1 -1
View File
@@ -101,7 +101,7 @@ Evaluate:
**STOP.** For each issue found in this section, call AskUserQuestion individually. One issue per call. Present options, state your recommendation, explain WHY. Do NOT batch multiple issues into one AskUserQuestion. Only proceed to the next section after ALL issues in this section are resolved.
### 3. Test review
Make a diagram of all new UX, new data flow, new codepaths, and new branching if statements or outcomes. For each, note what is new about the features discussed in this branch and plan. Then, for each new item in the diagram, make sure there is a JS or Rails test.
Make a diagram of all new UX, new data flow, new codepaths, and new branching if statements or outcomes. For each, note what is new about the features discussed in this branch and plan. Then, for each new item in the diagram, make sure there is a corresponding test.
For LLM/prompt changes: check the "Prompt/LLM changes" file patterns listed in CLAUDE.md. If this plan touches ANY of those patterns, state which eval suites must be run, which cases should be added, and what baselines to compare against. Then use AskUserQuestion to confirm the eval scope with the user.