feat(02-01): rewrite setup.sh with dependency checker, OS detection, color output, CLI args#8
Conversation
…Port Phase 10: Template Engine & Dividend Topic Port - Implementation decisions documented - Phase boundary established Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds ten new setup.sh helper functions for step-based, idempotent setup and more robust dependency version parsing; expands topic-schema knowledge enum to more states; updates Phase 10–11 planning and context docs for standalone explorers, persistence, and learning modes; removes/adds repo notes and deletes Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.planning/phases/02-setup-automation/02-02-SUMMARY.md:
- Line 80: The summary line incorrectly states "Added 8 new functions" while
listing 10 function names; update the sentence to reflect the actual count
(change "8" to "10") or remove/adjust the listed names so the number matches;
specifically edit the summary text that mentions `setup.sh` and the function
list including `is_step_complete`, `mark_step_complete`, `show_progress`,
`create_dir`, `create_directory_structure`, `verify_directory_structure`,
`scaffold_file`, `scaffold_config_files`, `install_python_deps`, and
`print_summary` so the numeric claim matches the enumerated functions.
In @.planning/phases/11-self-assessment-persistence-topics/11-01-PLAN.md:
- Around line 120-130: The markdown table under "**categories (7):**" has
inconsistent column counts; ensure every row matches the header "| ID | Label |
Color |" by having the same number of '|' separators (one leading, one trailing
and one between each column) for all entries (e.g., "| core-greeks | Core Greeks
| `#58a6ff` |"); update any rows missing the trailing pipe or with extra/missing
pipes so all rows have three columns and four pipe characters total.
In @.planning/phases/11-self-assessment-persistence-topics/11-02-PLAN.md:
- Line 918: The fenced code block using triple backticks is missing a language
which triggers MD040/MD046; update that fenced block by adding an explicit
language hint (for example `bash`) after the opening ``` or convert the block to
an indented code block style so the linter accepts it; ensure the opening fence
is changed from ``` to ```bash (or replace with four-space indentation) and keep
the block content unchanged.
In @.planning/phases/11-self-assessment-persistence-topics/11-03-PLAN.md:
- Line 345: The fenced code block shown in the diff is missing a language tag
which triggers MD040/MD046; update that fenced block by adding a language
identifier (for example use ```bash) or convert the block to an indented code
block so the linter treats it as code; ensure the fence opening and closing
markers remain consistent and that any leading/trailing whitespace is preserved.
In @.planning/phases/11-self-assessment-persistence-topics/11-CONTEXT.md:
- Around line 16-20: The heading "Knowledge State Cycling" is a level-3 (###)
header causing a markdownlint MD001 level jump; update that header (and any
sibling headers in this block) to level-2 (##) or adjust the preceding heading
so headings increment by one, ensuring "Knowledge State Cycling" and related
peer section headings use consistent levels to satisfy MD001.
In `@setup.sh`:
- Around line 214-220: The version-extraction case may leave found_version empty
if parsing fails; add a sentinel fallback so found_version is set to "0.0" when
empty. After the case that sets found_version (the block handling cmd values
python3, uv, bun), add a check like: if found_version is empty (e.g., [ -z
"$found_version" ]), assign found_version="0.0"; this ensures predictable
behavior under set -e and avoids downstream failures.
🧹 Nitpick comments (3)
scripts/qa/pii-replacements.txt (1)
19-19: Tighten the account-number regex to avoid partial matches.
Z0\d{7,}can match within longer tokens, leaving trailing characters unredacted. Consider word boundaries to limit partial replacements.♻️ Proposed refinement
-regex:Z0\d{7,}==>REDACTED_ACCOUNT +regex:\bZ0\d{7,}\b==>REDACTED_ACCOUNTsetup.sh (2)
27-27: Unused variableBLUE.The
BLUEvariable is declared but never used in the script. Consider removing it or using it for consistency if other output needs blue coloring.🧹 Optional: Remove unused variable
- BLUE='\033[0;34m'And remove the corresponding empty assignment on line 34:
- BLUE=''
400-404: Unused parameterdescription.The
descriptionparameter is declared but never used in the function body. Either remove it or use it in the prompt/logging.🧹 Option 1: Remove unused parameter
scaffold_file() { local target="$1" - local description="$2" local basename_targetAnd update callers to pass only one argument.
🧹 Option 2: Use description in prompt
if [ -t 0 ]; then - printf " %s already exists. Overwrite? [y/N] " "$basename_target" + printf " %s (%s) already exists. Overwrite? [y/N] " "$basename_target" "$description"
| **categories (7):** | ||
| | ID | Label | Color | | ||
| |----|-------|-------| | ||
| | core-greeks | Core Greeks | #58a6ff | | ||
| | option-fundamentals | Option Fundamentals | #3fb950 | | ||
| | time-effects | Time Effects | #d29922 | | ||
| | volatility-effects | Volatility Effects | #f85149 | | ||
| | strategy-impact | Strategy Impact | #bc8cff | | ||
| | second-order-greeks | Second-Order Greeks | #f778ba | | ||
| | volatility-surface | Volatility Surface | #ff9f43 | | ||
|
|
There was a problem hiding this comment.
Normalize table column counts (MD056).
The category tables around this section have inconsistent column counts. Ensure each row has the same number of | separators as the header so markdownlint doesn’t drop extra cells.
🤖 Prompt for AI Agents
In @.planning/phases/11-self-assessment-persistence-topics/11-01-PLAN.md around
lines 120 - 130, The markdown table under "**categories (7):**" has inconsistent
column counts; ensure every row matches the header "| ID | Label | Color |" by
having the same number of '|' separators (one leading, one trailing and one
between each column) for all entries (e.g., "| core-greeks | Core Greeks |
`#58a6ff` |"); update any rows missing the trailing pipe or with extra/missing
pipes so all rows have three columns and four pipe characters total.
| for pattern in "KNOWLEDGE_STATES" "localStorage" "LEARNING_MODES" "clipboard" "fallbackCopy" "migrateKnowledgeState" "debouncedSave" "exportStateAsJSON" "updateProgress" "restoreState" "mode-selector" "touch-action" "progress-fill" "max-width: 768px"; do | ||
| grep -rq "$pattern" src/explorer/template/ && echo " $pattern: YES" || echo " $pattern: NO" | ||
| done | ||
| ``` |
There was a problem hiding this comment.
Fix verification code block formatting (MD040/MD046).
Line 918 uses a fenced block without a language and the linter expects indented style. Add a language (e.g., bash) and/or convert to indented code block to match the configured style.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 918-918: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 918-918: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
In @.planning/phases/11-self-assessment-persistence-topics/11-02-PLAN.md at line
918, The fenced code block using triple backticks is missing a language which
triggers MD040/MD046; update that fenced block by adding an explicit language
hint (for example `bash`) after the opening ``` or convert the block to an
indented code block style so the linter accepts it; ensure the opening fence is
changed from ``` to ```bash (or replace with four-space indentation) and keep
the block content unchanged.
| console.log(' ${explorer}: ' + d.nodes.length + ' nodes, ' + d.edges.length + ' edges, slug=' + d.metadata.slug); | ||
| " | ||
| done | ||
| ``` |
There was a problem hiding this comment.
Fix verification code block formatting (MD040/MD046).
The fenced block at Line 345 lacks a language tag and the linter expects indented code blocks. Add a language (e.g., bash) and/or switch to indented formatting.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 345-345: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 345-345: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
In @.planning/phases/11-self-assessment-persistence-topics/11-03-PLAN.md at line
345, The fenced code block shown in the diff is missing a language tag which
triggers MD040/MD046; update that fenced block by adding a language identifier
(for example use ```bash) or convert the block to an indented code block so the
linter treats it as code; ensure the fence opening and closing markers remain
consistent and that any leading/trailing whitespace is preserved.
.planning/phases/11-self-assessment-persistence-topics/11-CONTEXT.md
Outdated
Show resolved
Hide resolved
- Fix function count mismatch in 02-02-SUMMARY.md (8 -> 10) - Fix MD001 heading level jumps in 11-CONTEXT.md (### -> ##) - Add version extraction fallback sentinel "0.0" in setup.sh Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 11: Self-Assessment, Persistence & Additional Topics - 3 plans in 2 waves - Wave 1: Plan 01 (schema + topic JSONs) + Plan 02 (template enhancements) - parallel - Wave 2: Plan 03 (build + human verification) - depends on both - Ready for execution https://claude.ai/code/session_01L2gGupsfqX9ooSgHVn2FY9
Expand options-greeks from 20 to 30 nodes (second-order Greeks, volatility surface) Expand risk-management from 20 to 27 nodes (advanced risk analytics, margin risk) Update Plan 11-03 verification scripts and human checklist for new counts https://claude.ai/code/session_01L2gGupsfqX9ooSgHVn2FY9
- Fix function count mismatch in 02-02-SUMMARY.md (8 -> 10) - Fix MD001 heading level jumps in 11-CONTEXT.md (### -> ##) - Add version extraction fallback sentinel "0.0" in setup.sh Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
9ac8e24 to
0bf292e
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
- Tighten account-number regex with word boundaries to prevent partial matches - Remove unused BLUE color variable from setup.sh - Wire scaffold_file description parameter into overwrite prompt Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
https://claude.ai/code/session_01Povov7P1xMFgnNG4V2Jydf
Summary by CodeRabbit
New Features
Bug Fixes
Documentation