feat: add demo-creator skill#979
Conversation
Add `.claude/skills/demo-creator/`, a skill that turns a one-line problem statement into a REAL recorded terminal demo plus a blog showing how to do it with `altimate-code` and why that's worth it. Highlights: - Two modes: `showcase` (default, `altimate-code` alone) and optional `comparison` (vs `claude-code`); a baseline is not required. - Two integrity laws: recordings reconcile with the session trace/`--format json` (no faking); value is shown, not asserted. - Real-run recording via `asciinema` + `agg` (`record.sh`), with `vhs` as a scripted fallback; frame extraction + authenticity cross-check (`inspect.sh`). - `extract_learnings.sh` mines a session into `learnings.md` (commands run, value-moment evidence, reusable lessons) feeding the blog and a lesson ledger. - `build_artifact.sh` inlines clips as base64 and renders headless so the page can be visually inspected before publishing as a Claude artifact. - Reuses `blog-writer`, `viral-tech-blog`, `humanizer`; `quality-bar.md` gate requires rendering and inspecting the published page, not just the clips. Validated end-to-end by dogfooding it on a PySpark SQL-port demo (artifacts kept out of this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughThis PR adds a new Claude skill called "demo-creator" under ChangesDemo-creator skill addition
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| mimetypes.add_type("image/svg+xml", ".svg") | ||
| inlined = [0]; missing = [] | ||
| def datauri(rel): | ||
| p = os.path.normpath(os.path.join(base, rel)) |
There was a problem hiding this comment.
[SUGGESTION]: Path-containment guard is a raw string prefix match
p.startswith(os.path.abspath(base)) treats the demo dir as a plain string prefix, so a relative image src that escapes into a sibling directory whose name extends base's name passes the guard (e.g. base .../demos/top, src=../topsecret/x.png -> .../demos/topsecret/x.png, which startswith(.../demos/top)). Since this check is explicitly meant to confine inlining to the demo dir, use a path-aware comparison instead of a string prefix:
| p = os.path.normpath(os.path.join(base, rel)) | |
| if not (os.path.commonpath([os.path.abspath(p), os.path.abspath(base)]) == os.path.abspath(base) and os.path.isfile(p)): |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| REPO_ROOT="$(cd "$SKILL_DIR/../../.." && pwd)" | ||
|
|
||
| demo_dir() { # demo_dir <topic-slug> | ||
| echo "$REPO_ROOT/demos/$1" |
There was a problem hiding this comment.
[SUGGESTION]: demo_dir doesn't sanitize the topic slug
$1 is interpolated directly into a path, and the result feeds mkdir -p, tee, and file writes across all the scripts. A slug containing .. (e.g. a malformed or agent-produced slug) resolves outside demos/ (e.g. topic a/../../b -> $REPO_ROOT/b). Consider rejecting non-slug characters so paths can't escape the demos dir:
| echo "$REPO_ROOT/demos/$1" | |
| case "$1" in *[![:alnum:]-]*|'') echo "demo_dir: invalid topic slug '$1'" >&2; return 1;; esac; echo "$REPO_ROOT/demos/$1" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| reset_fixture | ||
| M_ARGS=(); [ -n "$MODEL" ] && M_ARGS+=(--model "$MODEL") | ||
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | ||
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
There was a problem hiding this comment.
[SUGGESTION]: Empty-array expansion under set -u fails on bash < 4.4
lib.sh enables set -u. When --model is omitted, M_ARGS=() stays empty and "${M_ARGS[@]}" throws M_ARGS[@]: unbound variable on bash 3.2 (the macOS default /usr/bin/bash). These scripts target macOS (brew / Chrome.app / iTerm are referenced), so the common no---model invocation would abort there. The same applies to B_ARGS on line 67. Expand defensively:
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true | |
| ${M_ARGS[@]+"${M_ARGS[@]}"} "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (14 files)
Fix these issues in Kilo Cloud Reviewed by glm-5.2 · Input: 78.9K · Output: 29K · Cached: 486.1K |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e7c950fc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| B_ARGS=(); [ -n "$BASE_AGENT" ] && B_ARGS+=(--agent "$BASE_AGENT") | ||
| M_ARGS=(); [ -n "$MODEL" ] && M_ARGS+=(--model "$MODEL") | ||
| if [ -n "$BASE_DENY" ]; then | ||
| export OPENCODE_PERMISSION="{\"tools\":{$(echo "$BASE_DENY" | awk -F, '{for(i=1;i<=NF;i++){printf "%s\"%s\":\"deny\"", (i>1?",":""), $i}}')}}" |
There was a problem hiding this comment.
When --baseline-mode disable --baseline-deny ... is used, this exports OPENCODE_PERMISSION as {"tools":{"bash":"deny"}}. I checked the config loader and OPENCODE_PERMISSION is merged directly into result.permission, with each top-level key interpreted as the permission name, so this creates a rule for permission tools with pattern bash instead of denying bash. The capability-disabled baseline therefore still has the tools it was meant to remove, invalidating the comparison.
Useful? React with 👍 / 👎.
| M_ARGS=(); [ -n "$MODEL" ] && M_ARGS+=(--model "$MODEL") | ||
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | ||
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
There was a problem hiding this comment.
When callers follow the skill's comparison example without --model, this altimate run leaves M_ARGS empty and uses the configured/default model, while the baseline run above always passes --model "$BASE_MODEL" (default sonnet). That breaks the same-model control the script claims to enforce, so a win/loss can come from model selection rather than altimate-code; require an explicit model or derive matching values for both sides.
Useful? React with 👍 / 👎.
| ## Baseline: plain Claude Code | ||
|
|
||
| {{What happened when we ran the same prompt through claude-code (`claude -p`), same model. | ||
| Embed the baseline clip. Be fair — show what it got right too.}} | ||
|
|
There was a problem hiding this comment.
Showcase mode is the default and does not produce <angle>.baseline.gif, but the shared blog template always asks the author to describe a plain-Claude baseline and embeds that GIF. For the default path this either leaves a broken image in blog.md or pressures the author to invent/run a comparison that the skill explicitly says is optional; make this section conditional on comparison mode or provide a showcase-only template.
Useful? React with 👍 / 👎.
| `.cast`, then renders to `.gif` with **agg**. Capture the json too: | ||
| `altimate-code run --format json … > runs/<angle>.json` (authenticity proof). `vhs` is the |
There was a problem hiding this comment.
This tells users to record the GIF/cast first and then collect runs/<angle>.json from a second altimate-code run --format json invocation. For nondeterministic agent runs, that second run can use different tools or produce a different result, so the JSON no longer authenticates the clip and a mismatched recording can pass the law #1 gate. Capture the event stream or trace id from the exact recorded invocation instead of re-running.
Useful? React with 👍 / 👎.
| reset_fixture | ||
| M_ARGS=(); [ -n "$MODEL" ] && M_ARGS+=(--model "$MODEL") | ||
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | ||
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
There was a problem hiding this comment.
If the altimate run exits non-zero because of auth, model, CLI, or runtime errors, || true still lets the script finish and print that JSON was captured. That makes downstream phases treat a failed or partial run as a valid comparison result, which can corrupt the demo evidence; tolerate expected baseline failures separately, but the altimate run should fail the script when it does not complete.
Useful? React with 👍 / 👎.
| log "wrote $COUNT frames. Read them to verify legibility + the value moment." | ||
|
|
||
| # --- Authenticity cross-check pointers --- | ||
| JSON="$DD/runs/$ANGLE.json" |
There was a problem hiding this comment.
When inspecting a baseline clip via --prefix <angle>.baseline, this still reads runs/$ANGLE.json, which is the altimate run rather than runs/$ANGLE.baseline.json. In comparison mode the authenticity summary for the baseline frames is therefore from the wrong session, so a baseline clip can be accepted without reconciling with its own recorded run; switch the JSON path based on the inspected prefix or add an explicit baseline flag.
Useful? React with 👍 / 👎.
| DD="$(demo_dir "$TOPIC")"; RUNS="$DD/runs"; mkdir -p "$RUNS" | ||
|
|
||
| reset_fixture() { | ||
| [ -n "$RESET_FROM" ] || return 0 |
There was a problem hiding this comment.
Because --reset-from is optional, this no-ops when it is omitted; the baseline run then operates directly in --cwd, can edit the fixture, and the altimate run starts from those mutated files. That violates the script's “same fixture” control and can make altimate look better or worse based on the baseline's changes, so comparison mode should require a pristine reset/copy or refuse to run without one.
Useful? React with 👍 / 👎.
| ( cd "$CWD" && asciinema rec --overwrite --window-size "${COLS}x${ROWS}" \ | ||
| --command "bash -lc $(printf '%q' "$INNER")" "$CAST" ) |
There was a problem hiding this comment.
This wrapper relies on asciinema rec's exit code to decide whether to continue to GIF rendering, but asciinema v3 documents rec --return as the option for propagating the recorded command's exit status. When the recorded altimate-code run fails, the script can still render and report success, letting a failed demo pass the record gate; add --return or check the recorded command status explicitly.
Useful? React with 👍 / 👎.
| JAVA_HOME="" | ||
| for cand in \ | ||
| "/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home" \ | ||
| "/opt/homebrew/opt/openjdk@11/libexec/openjdk.jdk/Contents/Home" \ | ||
| "/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home"; do |
There was a problem hiding this comment.
In environments with a compatible JAVA_HOME or a JDK installed outside Apple Silicon Homebrew paths, this resets JAVA_HOME and only searches /opt/homebrew, then falls through to brew install. That makes the PySpark fixture setup fail on Linux, Intel macOS, or already-configured shells even though Spark could run; check the existing JAVA_HOME/java before attempting Homebrew installation.
Useful? React with 👍 / 👎.
| PY | ||
|
|
||
| if [ "$RENDER" = "1" ]; then | ||
| CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" |
There was a problem hiding this comment.
When --render is requested on Linux/CI or on macOS installs exposed as google-chrome/chromium, this hard-coded app bundle path is not executable, so the script skips rendering and still exits successfully. Since the skill makes render-inspection mandatory before publishing, --render should discover a browser on PATH or fail instead of silently producing an uninspected artifact.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
.claude/skills/demo-creator/SKILL.md (1)
92-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The directory tree code block is missing a language tag, triggering markdownlint MD040.
- ``` + ```text🤖 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/demo-creator/SKILL.md at line 92, The fenced directory tree block in SKILL.md is missing a language specifier, which triggers markdownlint MD040. Update the markdown fence for that block to use a language tag such as text so the code block is properly annotated while keeping the existing content unchanged..claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (1)
62-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStatic analysis: split declaration from export to avoid masking the exit status.
Shellcheck flags this combined assign+export, which would hide a failing command substitution.
Proposed fix
- export OPENCODE_PERMISSION="{\"tools\":{$(echo "$BASE_DENY" | awk -F, '{for(i=1;i<=NF;i++){printf "%s\"%s\":\"deny\"", (i>1?",":""), $i}}')}}" + OPENCODE_PERMISSION="{\"tools\":{$(echo "$BASE_DENY" | awk -F, '{for(i=1;i<=NF;i++){printf "%s\"%s\":\"deny\"", (i>1?",":""), $i}}')}}" + export OPENCODE_PERMISSION🤖 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/demo-creator/scripts/baseline_vs_altimate.sh around lines 62 - 65, The combined assign-and-export in baseline_vs_altimate.sh can mask a failing command substitution and should be split. Update the logic around BASE_DENY and OPENCODE_PERMISSION so the JSON string is built in a separate assignment first, then exported on the next line, preserving the exit status from the awk/echo pipeline. Keep the fix localized to the shell block that logs baseline OPENCODE_PERMISSION.Source: Linters/SAST tools
.claude/skills/demo-creator/references/recording.md (1)
43-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
vhslanguage identifier to the fenced code block.The VHS tape example lacks a language specifier, which triggered a markdownlint warning. Adding
vhsafter the opening fence will satisfy the linter and may enable syntax highlighting.+```vhs
Require altimate-code
Output clips/.gif<details> <summary>🤖 Prompt for AI Agents</summary>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/demo-creator/references/recording.md around lines 43 - 53,
The fenced VHS example in the recording reference is missing a language
identifier, which triggers the markdownlint warning. Update the code block in
the recording example to use a vhs fence so the snippet is explicitly recognized
as VHS syntax; keep the existing content unchanged and make the same adjustment
wherever this example is shown.</details> <!-- cr-comment:v1:bfc3223c1f9c3c691b3223aa --> </blockquote></details> <details> <summary>.claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (1)</summary><blockquote> `16-29`: _🎯 Functional Correctness_ | _🔵 Trivial_ | _⚡ Quick win_ **Hardcoded Apple Silicon Homebrew prefix breaks on Intel Macs.** All JDK candidate paths and the install fallback assume `/opt/homebrew` (Apple Silicon). On Intel Macs, Homebrew installs to `/usr/local`, so detection will always miss existing JDKs and the post-install path at Line 28 will also be wrong. <details> <summary>♻️ Suggested fix using `brew --prefix`</summary> ```diff for cand in \ - "/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home" \ - "/opt/homebrew/opt/openjdk@11/libexec/openjdk.jdk/Contents/Home" \ - "/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home"; do + "$(brew --prefix openjdk@17 2>/dev/null)/libexec/openjdk.jdk/Contents/Home" \ + "$(brew --prefix openjdk@11 2>/dev/null)/libexec/openjdk.jdk/Contents/Home" \ + "$(brew --prefix openjdk@21 2>/dev/null)/libexec/openjdk.jdk/Contents/Home"; do [ -x "$cand/bin/java" ] || continue ... if [ -z "$JAVA_HOME" ]; then log "no Spark-3.5-compatible JDK (8/11/17/21) found; installing openjdk@17" brew install openjdk@17 - JAVA_HOME="/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home" + JAVA_HOME="$(brew --prefix openjdk@17)/libexec/openjdk.jdk/Contents/Home" fi🤖 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/demo-creator/scripts/setup_pyspark_fixture.sh around lines 16 - 29, The JDK discovery in setup_pyspark_fixture.sh is hardcoded to the Apple Silicon Homebrew prefix, so it fails on Intel Macs. Update the JAVA_HOME candidate lookup in the JDK selection loop to derive the Homebrew prefix dynamically with brew --prefix instead of assuming /opt/homebrew, and use that same prefix for the openjdk@17 fallback path after brew install. Keep the existing java_major and log-based selection logic in place, just make the candidate paths and installed JDK home path prefix-agnostic.
.claude/skills/demo-creator/scripts/extract_learnings.sh (1)
11-14: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBlanket
set +e +o pipefailmasks failures beyond the stated grep/head justification.The disable applies to the rest of the script, including the final write to
$OUT(Lines 54-82). If that write fails for any reason, the script still prints "wrote $OUT" and exits 0, silently reporting success on failure.Consider scoping suppression to just the extraction helpers (e.g. wrap individual best-effort calls with
|| true, as already done in several spots) and re-enablingset -ebefore the final write block, rather than disabling it for the whole script tail.🤖 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/demo-creator/scripts/extract_learnings.sh around lines 11 - 14, The blanket set +e +o pipefail in extract_learnings.sh is too broad and leaves the final output write path unprotected, so narrow the failure suppression to the best-effort extraction steps only. Keep the existing grep/head tolerance in the helper sections, but re-enable strict error handling before the final write block that produces $OUT and emits the success message, so any failure in the write/reporting path correctly aborts the script.Source: Linters/SAST tools
.claude/skills/demo-creator/templates/replicate.md.tmpl (1)
16-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame conditional-baseline gap as
blog.md.tmpl.This "Reproduce the baseline" section is also unconditional, but per the SKILL.md-documented showcase/comparison modes a baseline only applies in comparison mode. Worth the same
{{IF COMPARISON MODE}}annotation as suggested forblog.md.tmpl.🤖 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/demo-creator/templates/replicate.md.tmpl around lines 16 - 20, The “Reproduce the baseline” section in the replicate template is unconditional even though it should only appear for comparison mode. Update the `replicate.md.tmpl` conditional logic to wrap the baseline step with the same `{{IF COMPARISON MODE}}` style gating used for other templates, so the baseline command and expected output only render when comparison mode is active.
.claude/skills/demo-creator/scripts/build_artifact.sh (2)
25-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOutput directory isn't created defensively before writing.
demo_dir()(inlib.sh) just echoes$REPO_ROOT/demos/$1without creating it, andopen(out, "w")in the python block will raiseFileNotFoundErrorif$DD(or a nested--outpath) doesn't exist yet. In the documented workflow this is safe since earlier phases always populatedemos/$topicfirst, but amkdir -phere would make the script robust to being run standalone/out of order.Also applies to: 48-49
🤖 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/demo-creator/scripts/build_artifact.sh around lines 25 - 26, The artifact build flow does not defensively create the output directory before writing, so make the build step robust when run standalone or out of order. Update the build logic in build_artifact.sh around demo_dir/demo_dir-derived variables and the Python open(out, "w") path handling to ensure $DD and any nested $OUT path exist first, for example by creating the parent directory before writing. Keep the fix localized to the artifact build path so the existing demo_dir helper in lib.sh can remain a pure path resolver.
54-65: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRender step is macOS-only and doesn't verify the screenshot was actually produced.
The hardcoded Chrome path only works on macOS; on Linux/Windows the render silently no-ops (lines 57-64), and even when
$CHROMEexists, there's no check that$SHOTwas actually written before the success log/echo at lines 60-61 — a page-load failure underfile://would still report success since Chrome's headless screenshot can exit 0 without producing a usable image.♻️ Suggested improvement
if [ "$RENDER" = "1" ]; then - CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" + CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" + for c in "$CHROME" google-chrome google-chrome-stable chromium chromium-browser; do + command -v "$c" >/dev/null 2>&1 && CHROME="$c" && break + done SHOT="$DD/clips/frames/_artifact_render.png"; mkdir -p "$(dirname "$SHOT")" - if [ -x "$CHROME" ]; then + if command -v "$CHROME" >/dev/null 2>&1 || [ -x "$CHROME" ]; then "$CHROME" --headless=new --disable-gpu --hide-scrollbars --force-device-scale-factor=1 \ --window-size="$W,$H" --screenshot="$SHOT" "file://$OUTPATH" 2>/dev/null - log "rendered for inspection -> $SHOT (Read it; check empty grid cells, mostly-empty GIFs, no h-scroll)" - echo "$SHOT" + if [ -s "$SHOT" ]; then + log "rendered for inspection -> $SHOT (Read it; check empty grid cells, mostly-empty GIFs, no h-scroll)" + echo "$SHOT" + else + log "WARNING: render did not produce $SHOT; open $OUTPATH manually to inspect." + fi else🤖 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/demo-creator/scripts/build_artifact.sh around lines 54 - 65, The render step in build_artifact.sh is macOS-only and assumes the screenshot succeeded even when Chrome returns without writing the image. Update the RENDER block around the CHROME/SHOT logic to use a cross-platform browser lookup or configurable Chrome path instead of a hardcoded macOS binary, and after invoking the headless screenshot path, verify that the screenshot file referenced by SHOT exists and is non-empty before logging success or echoing it. Keep the checks and messages anchored to the existing RENDER, CHROME, and SHOT flow so failures are reported instead of silently passing.
.claude/skills/demo-creator/templates/blog.md.tmpl (1)
19-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBaseline section is presented as unconditional, but the workflow defines it as comparison-mode-only.
SKILL.md's Phase 7 explicitly states baseline contrast is only added "In comparison mode... in showcase mode, the value stands on its own," per the SKILL.md snippet for this file. The template doesn't annotate the "Baseline: plain Claude Code" section as conditional, so an author following the skeleton literally could include it even in showcase mode. Consider adding a
{{IF COMPARISON MODE — omit this section in showcase mode}}marker around lines 19-26.🤖 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/demo-creator/templates/blog.md.tmpl around lines 19 - 26, The “Baseline: plain Claude Code” block in the blog template is currently written as if it always applies, but it should only appear in comparison mode. Update the blog.md.tmpl section anchored by the “Baseline: plain Claude Code” heading so it is clearly conditional, using the same template conventions as other mode-specific content in this file, and make sure the surrounding placeholders and caption text indicate it must be omitted in showcase mode.
🤖 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/demo-creator/scripts/baseline_vs_altimate.sh:
- Around line 43-48: The fixture reset in reset_fixture is currently optional,
which can leave the altimate run starting from the baseline run’s modified state
instead of the same fixture. Update baseline_vs_altimate.sh so reset_fixture
either requires --reset-from or emits a loud warning when RESET_FROM is unset,
and make sure the comparison path around the baseline and altimate invocations
uses the same initial fixture state.
- Line 57: The baseline_vs_altimate.sh pipeline currently masks failures with
the trailing || true on the claude/altimate-code run and tee steps, so the
script can continue and print the json captured message even when a run fails.
Update the run handling around the baseline and altimate execution blocks to
capture the command exit status explicitly, propagate or report non-zero
failures, and only emit the success message after both runs complete
successfully. Use the existing run pipeline and the json captured logging path
as the main anchors for the fix.
In @.claude/skills/demo-creator/scripts/build_artifact.sh:
- Around line 34-40: The path validation in datauri is using a raw string prefix
check, which can be bypassed by sibling directories that share the same leading
characters. Tighten the guard in datauri by validating the resolved path against
the absolute base directory with a proper path-boundary aware check, and keep
the existing os.path.isfile condition so only files under the intended base are
inlined.
In @.claude/skills/demo-creator/scripts/record.sh:
- Line 29: The record.sh dependency check only verifies that the asciinema
binary exists, so older versions can still fail later when `record.sh` calls
asciinema with `--window-size`. Update the `need()`/startup validation around
the `asciinema` check to verify the installed asciinema is v3 or newer before
continuing, and exit early with a clear error message if the version is too old.
Keep the fix localized to the existing `need` usage and the `asciinema`
invocation path so the script never reaches the unsupported CLI flag.
In @.claude/skills/demo-creator/templates/blog.md.tmpl:
- Line 45: The blog template and replicate template are using inconsistent
filename casing, which can make the generated docs diverge or break on
case-sensitive links. Update the wording in the blog section that references
REPLICATE.md so it matches the actual template name used by the demo creator,
and keep the related identifiers in sync with the replicate template content to
avoid casing mismatches between blog.md.tmpl and replicate.md.tmpl.
---
Nitpick comments:
In @.claude/skills/demo-creator/references/recording.md:
- Around line 43-53: The fenced VHS example in the recording reference is
missing a language identifier, which triggers the markdownlint warning. Update
the code block in the recording example to use a vhs fence so the snippet is
explicitly recognized as VHS syntax; keep the existing content unchanged and
make the same adjustment wherever this example is shown.
In @.claude/skills/demo-creator/scripts/baseline_vs_altimate.sh:
- Around line 62-65: The combined assign-and-export in baseline_vs_altimate.sh
can mask a failing command substitution and should be split. Update the logic
around BASE_DENY and OPENCODE_PERMISSION so the JSON string is built in a
separate assignment first, then exported on the next line, preserving the exit
status from the awk/echo pipeline. Keep the fix localized to the shell block
that logs baseline OPENCODE_PERMISSION.
In @.claude/skills/demo-creator/scripts/build_artifact.sh:
- Around line 25-26: The artifact build flow does not defensively create the
output directory before writing, so make the build step robust when run
standalone or out of order. Update the build logic in build_artifact.sh around
demo_dir/demo_dir-derived variables and the Python open(out, "w") path handling
to ensure $DD and any nested $OUT path exist first, for example by creating the
parent directory before writing. Keep the fix localized to the artifact build
path so the existing demo_dir helper in lib.sh can remain a pure path resolver.
- Around line 54-65: The render step in build_artifact.sh is macOS-only and
assumes the screenshot succeeded even when Chrome returns without writing the
image. Update the RENDER block around the CHROME/SHOT logic to use a
cross-platform browser lookup or configurable Chrome path instead of a hardcoded
macOS binary, and after invoking the headless screenshot path, verify that the
screenshot file referenced by SHOT exists and is non-empty before logging
success or echoing it. Keep the checks and messages anchored to the existing
RENDER, CHROME, and SHOT flow so failures are reported instead of silently
passing.
In @.claude/skills/demo-creator/scripts/extract_learnings.sh:
- Around line 11-14: The blanket set +e +o pipefail in extract_learnings.sh is
too broad and leaves the final output write path unprotected, so narrow the
failure suppression to the best-effort extraction steps only. Keep the existing
grep/head tolerance in the helper sections, but re-enable strict error handling
before the final write block that produces $OUT and emits the success message,
so any failure in the write/reporting path correctly aborts the script.
In @.claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh:
- Around line 16-29: The JDK discovery in setup_pyspark_fixture.sh is hardcoded
to the Apple Silicon Homebrew prefix, so it fails on Intel Macs. Update the
JAVA_HOME candidate lookup in the JDK selection loop to derive the Homebrew
prefix dynamically with brew --prefix instead of assuming /opt/homebrew, and use
that same prefix for the openjdk@17 fallback path after brew install. Keep the
existing java_major and log-based selection logic in place, just make the
candidate paths and installed JDK home path prefix-agnostic.
In @.claude/skills/demo-creator/SKILL.md:
- Line 92: The fenced directory tree block in SKILL.md is missing a language
specifier, which triggers markdownlint MD040. Update the markdown fence for that
block to use a language tag such as text so the code block is properly annotated
while keeping the existing content unchanged.
In @.claude/skills/demo-creator/templates/blog.md.tmpl:
- Around line 19-26: The “Baseline: plain Claude Code” block in the blog
template is currently written as if it always applies, but it should only appear
in comparison mode. Update the blog.md.tmpl section anchored by the “Baseline:
plain Claude Code” heading so it is clearly conditional, using the same template
conventions as other mode-specific content in this file, and make sure the
surrounding placeholders and caption text indicate it must be omitted in
showcase mode.
In @.claude/skills/demo-creator/templates/replicate.md.tmpl:
- Around line 16-20: The “Reproduce the baseline” section in the replicate
template is unconditional even though it should only appear for comparison mode.
Update the `replicate.md.tmpl` conditional logic to wrap the baseline step with
the same `{{IF COMPARISON MODE}}` style gating used for other templates, so the
baseline command and expected output only render when comparison mode is active.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2c0d7a5b-cef4-44f9-9eb2-18bf6d87a950
📒 Files selected for processing (14)
.claude/skills/demo-creator/SKILL.md.claude/skills/demo-creator/references/quality-bar.md.claude/skills/demo-creator/references/recording.md.claude/skills/demo-creator/references/value-wedge.md.claude/skills/demo-creator/scripts/baseline_vs_altimate.sh.claude/skills/demo-creator/scripts/build_artifact.sh.claude/skills/demo-creator/scripts/extract_learnings.sh.claude/skills/demo-creator/scripts/inspect.sh.claude/skills/demo-creator/scripts/lib.sh.claude/skills/demo-creator/scripts/record.sh.claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh.claude/skills/demo-creator/templates/blog.md.tmpl.claude/skills/demo-creator/templates/demo.tape.tmpl.claude/skills/demo-creator/templates/replicate.md.tmpl
| need claude | ||
| ( cd "$CWD" && claude -p --dangerously-skip-permissions --max-turns "$MAXT" \ | ||
| --output-format json --model "$BASE_MODEL" "$PROMPT" ) \ | ||
| | tee "$RUNS/$ANGLE.baseline.json" || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
|| true silently hides run failures, but the script still reports success.
Each run's pipeline (... | tee ... || true) absorbs any failure from claude or altimate-code run under set -euo pipefail. If either invocation crashes, hits an auth error, or is killed, the script proceeds straight to Line 78's "json captured" message as if both runs succeeded, even though one JSON file may be empty or contain a partial/error payload. This risks a demo author building on a broken comparison without realizing it failed.
Consider capturing the exit status and surfacing it explicitly.
Proposed fix: track and report run exit status
- ( cd "$CWD" && claude -p --dangerously-skip-permissions --max-turns "$MAXT" \
- --output-format json --model "$BASE_MODEL" "$PROMPT" ) \
- | tee "$RUNS/$ANGLE.baseline.json" || true
+ ( cd "$CWD" && claude -p --dangerously-skip-permissions --max-turns "$MAXT" \
+ --output-format json --model "$BASE_MODEL" "$PROMPT" ) \
+ | tee "$RUNS/$ANGLE.baseline.json"
+ status=$?
+ [ "$status" -eq 0 ] || log "WARNING: baseline run exited with status $status"Also applies to: 67-67, 76-76, 78-78
🤖 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/demo-creator/scripts/baseline_vs_altimate.sh at line 57, The
baseline_vs_altimate.sh pipeline currently masks failures with the trailing ||
true on the claude/altimate-code run and tee steps, so the script can continue
and print the json captured message even when a run fails. Update the run
handling around the baseline and altimate execution blocks to capture the
command exit status explicitly, propagate or report non-zero failures, and only
emit the success message after both runs complete successfully. Use the existing
run pipeline and the json captured logging path as the main anchors for the fix.
| done | ||
| [ $# -ge 1 ] || { echo "ERROR: no command after --" >&2; exit 2; } | ||
|
|
||
| need asciinema asciinema; need agg agg |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
need asciinema asciinema
case "$(asciinema --version 2>/dev/null)" in
*" 3."*|*" 3,"*) ;;
*) echo "ERROR: asciinema v3+ required (found: $(asciinema --version 2>/dev/null))" >&2; exit 1;;
esacRepository: AltimateAI/altimate-code
Length of output: 240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target script and nearby helpers
git ls-files '.claude/skills/demo-creator/scripts/record.sh' '.claude/skills/demo-creator/scripts/*' '.claude/skills/demo-creator/**' | sed -n '1,120p'
echo
echo "=== record.sh excerpt ==="
nl -ba .claude/skills/demo-creator/scripts/record.sh | sed -n '1,140p'
echo
echo "=== search for need() definition/usages ==="
rg -n '(^|[[:space:]])need[[:space:]]*\(' .claude -g '*.sh' -g '*.bash' -g '*.zsh' || true
rg -n 'need[[:space:]]+asciinema|asciinema --version|--window-size|--cols|--rows' .claude -g '*.sh' -g '*.md' || trueRepository: AltimateAI/altimate-code
Length of output: 954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If a helper defines `need`, inspect it directly.
for f in $(git ls-files '.claude/**/*.sh' '.claude/**/*.bash' '.claude/**/*.zsh'); do
if rg -n '(^|[[:space:]])need[[:space:]]*\(' "$f" >/dev/null; then
echo "=== $f ==="
nl -ba "$f" | sed -n '1,220p'
fi
doneRepository: AltimateAI/altimate-code
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== target file ==="
nl -ba .claude/skills/demo-creator/scripts/record.sh | sed -n '1,140p'
echo
echo "=== helper searches ==="
rg -n '(^|[[:space:]])need[[:space:]]*\(' .claude -g '*.sh' -g '*.bash' -g '*.zsh' || true
rg -n 'need[[:space:]]+asciinema|asciinema --version|--window-size|--cols|--rows' .claude -g '*.sh' -g '*.md' || trueRepository: AltimateAI/altimate-code
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== record.sh (lines 1-160) ==="
sed -n '1,160p' .claude/skills/demo-creator/scripts/record.sh | cat -n
echo
echo "=== lib.sh (lines 1-220) ==="
sed -n '1,220p' .claude/skills/demo-creator/scripts/lib.sh | cat -n
echo
echo "=== searches ==="
rg -n '(^|[[:space:]])need[[:space:]]*\(' .claude/skills/demo-creator/scripts -g '*.sh' || true
rg -n 'need[[:space:]]+asciinema|asciinema --version|--window-size|--cols|--rows' .claude/skills/demo-creator -g '*.sh' -g '*.md' || trueRepository: AltimateAI/altimate-code
Length of output: 5020
Require an asciinema v3 check before calling --window-size need() only checks that asciinema exists; an older binary will still reach asciinema rec --window-size ... and fail with a CLI parse error. Add a version guard here so the script exits with a clear message.
🤖 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/demo-creator/scripts/record.sh at line 29, The record.sh
dependency check only verifies that the asciinema binary exists, so older
versions can still fail later when `record.sh` calls asciinema with
`--window-size`. Update the `need()`/startup validation around the `asciinema`
check to verify the installed asciinema is v3 or newer before continuing, and
exit early with a clear error message if the version is too old. Keep the fix
localized to the existing `need` usage and the `asciinema` invocation path so
the script never reaches the unsupported CLI flag.
|
|
||
| ## Replicate it yourself | ||
|
|
||
| {{Exact, copy-pasteable steps — must match REPLICATE.md and the fixture scripts. A reader |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Casing mismatch: blog references REPLICATE.md, but the template file is replicate.md.tmpl.
The "Replicate it yourself" section says steps "must match REPLICATE.md" (uppercase), matching SKILL.md's Phase 7 wording, yet the corresponding template in this PR is named replicate.md.tmpl (lowercase). If both are taken literally, the blog and the generated replicate doc would diverge in filename casing, risking a broken link on case-sensitive filesystems/links.
🤖 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/demo-creator/templates/blog.md.tmpl at line 45, The blog
template and replicate template are using inconsistent filename casing, which
can make the generated docs diverge or break on case-sensitive links. Update the
wording in the blog section that references REPLICATE.md so it matches the
actual template name used by the demo creator, and keep the related identifiers
in sync with the replicate template content to avoid casing mismatches between
blog.md.tmpl and replicate.md.tmpl.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
18 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh">
<violation number="1" location=".claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh:37">
P2: `uv venv` failure is silently swallowed by `|| true`, which prevents the `python3 -m venv` fallback from ever running when uv itself exists but fails to create the venv.</violation>
</file>
<file name=".claude/skills/demo-creator/scripts/baseline_vs_altimate.sh">
<violation number="1" location=".claude/skills/demo-creator/scripts/baseline_vs_altimate.sh:30">
P2: Validate `--baseline-mode`; typos currently fall into disable mode and can make the baseline run with altimate-code capabilities.</violation>
<violation number="2" location=".claude/skills/demo-creator/scripts/baseline_vs_altimate.sh:44">
P2: When `--reset-from` is omitted, `reset_fixture()` is a no-op. The baseline run can then modify files in `--cwd`, and the altimate run starts from those mutated files rather than a pristine state. This violates the script's "same fixture" control and can make either side look better or worse based on the other's edits. Comparison mode should require `--reset-from` or refuse to run without it.</violation>
</file>
<file name=".claude/skills/demo-creator/scripts/extract_learnings.sh">
<violation number="1" location=".claude/skills/demo-creator/scripts/extract_learnings.sh:82">
P2: Check the main learnings write explicitly because `set +e` makes write failures non-fatal and the script can report success without creating the result.</violation>
<violation number="2" location=".claude/skills/demo-creator/scripts/extract_learnings.sh:92">
P2: Check the baseline append explicitly; otherwise `--baseline` can silently omit its contrast section under the script’s disabled errexit mode.</violation>
</file>
<file name=".claude/skills/demo-creator/SKILL.md">
<violation number="1" location=".claude/skills/demo-creator/SKILL.md:110">
P2: Hardcoded absolute path `/Users/anandgupta/obsidian/...` breaks on any other machine or if the vault path changes. Use `$OBSIDIAN_VAULT` env var or a config reference instead.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| --baseline-model) BASE_MODEL="$2"; shift 2;; | ||
| --reset-from) RESET_FROM="$2"; shift 2;; | ||
| --max-turns) MAXT="$2"; shift 2;; | ||
| --baseline-mode) BASE_MODE="$2"; shift 2;; # claude (default) | disable |
There was a problem hiding this comment.
P2: Validate --baseline-mode; typos currently fall into disable mode and can make the baseline run with altimate-code capabilities.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/demo-creator/scripts/baseline_vs_altimate.sh, line 30:
<comment>Validate `--baseline-mode`; typos currently fall into disable mode and can make the baseline run with altimate-code capabilities.</comment>
<file context>
@@ -0,0 +1,80 @@
+ --baseline-model) BASE_MODEL="$2"; shift 2;;
+ --reset-from) RESET_FROM="$2"; shift 2;;
+ --max-turns) MAXT="$2"; shift 2;;
+ --baseline-mode) BASE_MODE="$2"; shift 2;; # claude (default) | disable
+ --baseline-deny) BASE_DENY="$2"; shift 2;; # only for --baseline-mode disable
+ --baseline-agent) BASE_AGENT="$2"; shift 2;; # only for --baseline-mode disable
</file context>
| if [ -n "$BH" ]; then echo '```'; echo "$BH"; echo '```'; else | ||
| echo "(baseline shows none of: $VALUE_MARKERS — this absence IS the contrast.)" | ||
| fi | ||
| } >> "$OUT" |
There was a problem hiding this comment.
P2: Check the baseline append explicitly; otherwise --baseline can silently omit its contrast section under the script’s disabled errexit mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/demo-creator/scripts/extract_learnings.sh, line 92:
<comment>Check the baseline append explicitly; otherwise `--baseline` can silently omit its contrast section under the script’s disabled errexit mode.</comment>
<file context>
@@ -0,0 +1,96 @@
+ if [ -n "$BH" ]; then echo '```'; echo "$BH"; echo '```'; else
+ echo "(baseline shows none of: $VALUE_MARKERS — this absence IS the contrast.)"
+ fi
+ } >> "$OUT"
+fi
+
</file context>
dev-punia-altimate
left a comment
dev-punia-altimate
left a comment
There was a problem hiding this comment.
🤖 Code Review — OpenCodeReview (Gemini) — 18 finding(s)
- 18 anchored to a line (posted inline when the comment stream is on)
- 0 without a line anchor
All findings (full text)
1. .claude/skills/demo-creator/scripts/inspect.sh (L11-L17)
[🟠 MEDIUM] Because set -euo pipefail (specifically set -u) is enabled, if the script is invoked with --fps or --prefix as the very last argument without providing a corresponding value, evaluating $2 will throw an unbound variable error and crash the script ungracefully (e.g., ./inspect.sh: line 13: $2: unbound variable).
You can add a check to explicitly ensure that the argument value is present, making the script more robust.
Suggested change:
while [ $# -gt 0 ]; do
case "$1" in
--fps)
[ $# -ge 2 ] || { echo "--fps requires a value" >&2; exit 2; }
FPS="$2"; shift 2;;
--prefix)
[ $# -ge 2 ] || { echo "--prefix requires a value" >&2; exit 2; }
PREFIX="$2"; shift 2;;
*) echo "unknown opt: $1" >&2; exit 2;;
esac
done
2. .claude/skills/demo-creator/scripts/inspect.sh (L28)
[🟠 MEDIUM] Due to set -euo pipefail, if ls fails to find any matching files (e.g., if ffmpeg produces no output due to an empty or corrupted GIF), it will exit with a non-zero status. This will cause the entire pipeline to fail and the script will silently abort at this line without printing any error message since stderr is redirected to /dev/null.
Additionally, using shell glob expansion with ls can result in an Argument list too long error if a massive number of frames is extracted.
Using find avoids pipeline failures when no files exist (since find safely returns 0) and protects against the argument list limit.
Suggested change:
COUNT=$(find "$FRAMES" -maxdepth 1 -name "f_*.png" 2>/dev/null | wc -l | tr -d ' ')
3. .claude/skills/demo-creator/scripts/extract_learnings.sh (L33-L34)
[🔵 LOW] Since SRC is reassigned to the temporary file $CLEAN, $(basename "$SRC") later in the script will output the temporary filename (e.g., tmp.xxxxxx) instead of the original log or JSON filename.
Consider storing the original filename before reassignment to preserve traceability.
Suggested change:
perl -pe 's/\x1b\[[0-9;]*[a-zA-Z]//g' "$SRC" > "$CLEAN" 2>/dev/null || cp "$SRC" "$CLEAN"
ORIG_SRC="$SRC"
SRC="$CLEAN"
4. .claude/skills/demo-creator/scripts/extract_learnings.sh (L55-L57)
[🔵 LOW] Use the original source file name here to accurately report where the learnings were mined from.
Suggested change:
echo "# Learnings — $TOPIC / $ANGLE"
echo
echo "_Mined from \`$(basename "$ORIG_SRC")\`$( [ -n "$TRACE_ID" ] && echo " · trace \`$TRACE_ID\`")._"
5. .claude/skills/demo-creator/scripts/extract_learnings.sh (L42-L45)
[🟠 MEDIUM] The regex [^"]* will truncate the extracted command at the first double quote. While this correctly stops at the end of a JSON string property, it will prematurely truncate shell commands that themselves contain double quotes (e.g., python -c "print('test')" or escaped quotes like \").
Since this is explicitly a "best-effort miner", this might be an acceptable limitation, but if extracting complete command arguments is important, consider a more robust matching strategy or using a JSON parser like jq when SRC is a JSON file.
6. .claude/skills/demo-creator/scripts/record.sh (L49)
[🟠 MEDIUM] The use of bash -lc initiates a login shell, which will source user profile scripts (e.g., ~/.bash_profile, ~/.profile). This could unexpectedly output text (like a Message of the Day, or initialization logs) that gets captured in the asciinema recording, potentially corrupting the demonstration.
Unless login shell behaviors are strictly required (like loading custom $PATH from profile scripts), it is safer to use a non-login shell with bash -c.
Suggested change:
--command "bash -c $(printf '%q' "$INNER")" "$CAST" )
7. .claude/skills/demo-creator/scripts/record.sh (L14-L22)
[🟠 MEDIUM] There is no boundary check for the option arguments in this while loop. Because the script uses set -u, providing a flag (e.g., --cwd or --cols) as the very last argument without its value will trigger a confusing unbound variable error on $2 and abruptly exit.
To improve the user experience and provide clearer error messages, consider verifying that $2 exists before assignment, or using ${2:-}. For example:
Suggested change:
while [ $# -gt 0 ]; do
case "$1" in
--cwd) CWD="${2?Missing argument for --cwd}"; shift 2;;
--cols) COLS="${2?Missing argument for --cols}"; shift 2;;
--rows) ROWS="${2?Missing argument for --rows}"; shift 2;;
--font) FONT="${2?Missing argument for --font}"; shift 2;;
--idle) IDLE="${2?Missing argument for --idle}"; shift 2;;
--out-prefix) PREFIX="${2?Missing argument for --out-prefix}"; shift 2;;
--banner) BANNER="${2?Missing argument for --banner}"; shift 2;; # shown on screen before the run (headless hides the invocation)
8. .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (L16-L21)
[🔴 HIGH] Platform dependence and rigid JAVA_HOME handling. The script hardcodes Apple Silicon macOS paths (/opt/homebrew/...) for JDK discovery and strictly relies on brew to install openjdk if a suitable version is not found. This will cause failures on Linux, Windows, CI containers, and Intel Macs (/usr/local/opt/...). Additionally, it completely ignores any existing, valid JAVA_HOME environment variable provided by the user.
Consider including the user's $JAVA_HOME in the candidate list and generalizing the discovery logic.
Suggested change:
ENV_JAVA_HOME="${JAVA_HOME:-}"
JAVA_HOME=""
for cand in \
"$ENV_JAVA_HOME" \
"/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home" \
"/opt/homebrew/opt/openjdk@11/libexec/openjdk.jdk/Contents/Home" \
"/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home"; do
[ -n "$cand" ] || continue
[ -x "$cand/bin/java" ] || continue
9. .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (L15)
[🟠 MEDIUM] Incorrect Java major version parsing for Java 8. The java_major function uses a regex that incorrectly extracts 1 instead of 8 for Java 8 (e.g., java version "1.8.0_xxx"). As a result, the subsequent case statement will erroneously reject valid Java 8 installations and force an unnecessary JDK download.
You can adjust the regex to account for the 1.x pattern used by older Java versions.
Suggested change:
java_major() { "$1/bin/java" -version 2>&1 | sed -nE 's/.*version "(1\.)?([0-9]+).*/\2/p' | head -1; }
10. .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (L39-L41)
[🟠 MEDIUM] Missing prerequisite validation in the fallback path. When the uv tool is unavailable, the script silently falls back to executing python3 -m venv without verifying if the python3 command actually exists on the host. This will result in a messy 'command not found' failure mid-execution.
You can use the standard need helper from lib.sh to validate python3 presence.
Suggested change:
else
need python3
python3 -m venv "$VENV"; "$VENV/bin/pip" install -q --upgrade pip "$PYSPARK_SPEC"
fi
11. .claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (L66-L67)
[🔴 HIGH] Expanding empty arrays ("${M_ARGS[@]}" and "${B_ARGS[@]}") with set -u enabled will trigger an "unbound variable" error and crash the script on Bash versions older than 4.4 (which includes the default Bash 3.2 on macOS).
Consider using fallback syntax "${M_ARGS[@]+"${M_ARGS[@]}"}" or checking for length before expansion to ensure cross-compatibility.
Suggested change:
( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \
${M_ARGS[@]+"${M_ARGS[@]}"} ${B_ARGS[@]+"${B_ARGS[@]}"} "$PROMPT" ) | tee "$RUNS/$ANGLE.baseline.json" || true
12. .claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (L62-L68)
[🟠 MEDIUM] The OPENCODE_PERMISSION variable is conditionally exported in the main script environment and then unconditionally cleared via unset OPENCODE_PERMISSION.
If a user had this variable configured in their environment prior to running the script, it will be unintentionally wiped out for the remainder of the script (e.g., during the subsequent ALTIMATE run).
To avoid side effects, consider moving the conditional export logic inside the ( ... ) subshell so it naturally scopes to the baseline run without affecting the parent environment.
Suggested change:
(
if [ -n "$BASE_DENY" ]; then
export OPENCODE_PERMISSION="{\"tools\":{$(echo "$BASE_DENY" | awk -F, '{for(i=1;i<=NF;i++){printf "%s\"%s\":\"deny\"", (i>1?",":""), $i}}')}}"
log " baseline OPENCODE_PERMISSION=$OPENCODE_PERMISSION"
fi
cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \
${M_ARGS[@]+"${M_ARGS[@]}"} ${B_ARGS[@]+"${B_ARGS[@]}"} "$PROMPT"
) | tee "$RUNS/$ANGLE.baseline.json" || true
13. .claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (L23-L33)
[🟠 MEDIUM] Argument parsing directly accesses $2 and shifts by 2 for flags that expect values.
Since set -u is enabled, if any of these flags are passed as the very last argument without a following value, the script will crash abruptly with a Bash "unbound variable" error rather than printing a helpful validation message.
Consider checking $# -ge 2 before processing these flags, or using standard fallback syntax like ${2:-}.
Suggested change:
case "$1" in
--cwd) [ $# -ge 2 ] || { echo "ERROR: --cwd requires a value" >&2; exit 2; }; CWD="$2"; shift 2;;
--prompt) [ $# -ge 2 ] || { echo "ERROR: --prompt requires a value" >&2; exit 2; }; PROMPT="$2"; shift 2;;
--model) [ $# -ge 2 ] || { echo "ERROR: --model requires a value" >&2; exit 2; }; MODEL="$2"; shift 2;;
--baseline-model) [ $# -ge 2 ] || { echo "ERROR: --baseline-model requires a value" >&2; exit 2; }; BASE_MODEL="$2"; shift 2;;
--reset-from) [ $# -ge 2 ] || { echo "ERROR: --reset-from requires a value" >&2; exit 2; }; RESET_FROM="$2"; shift 2;;
--max-turns) [ $# -ge 2 ] || { echo "ERROR: --max-turns requires a value" >&2; exit 2; }; MAXT="$2"; shift 2;;
--baseline-mode) [ $# -ge 2 ] || { echo "ERROR: --baseline-mode requires a value" >&2; exit 2; }; BASE_MODE="$2"; shift 2;;
--baseline-deny) [ $# -ge 2 ] || { echo "ERROR: --baseline-deny requires a value" >&2; exit 2; }; BASE_DENY="$2"; shift 2;;
--baseline-agent) [ $# -ge 2 ] || { echo "ERROR: --baseline-agent requires a value" >&2; exit 2; }; BASE_AGENT="$2"; shift 2;;
-*) echo "unknown opt: $1" >&2; exit 2;;
14. .claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (L75-L76)
[🔴 HIGH] This array expansion will also trigger an "unbound variable" error under Bash 3.2 when the array is empty. Consider applying the same fallback syntax here.
Suggested change:
( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \
${M_ARGS[@]+"${M_ARGS[@]}"} "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true
15. .claude/skills/demo-creator/scripts/build_artifact.sh (L16-L22)
[🟠 MEDIUM] Since set -u is enabled at the top of the script, referencing $2 without checking if it exists will throw an abrupt bash "unbound variable" error if an option like --out is passed as the very last argument. Additionally, shift 2 will fail if there are less than 2 arguments left. It's safer to explicitly check $# to provide a clear error message.
Suggested change:
while [ $# -gt 0 ]; do case "$1" in
--out) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; OUT="$2"; shift 2;;
--render) RENDER=1; shift;;
--width) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; W="$2"; shift 2;;
--height) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; H="$2"; shift 2;;
*) echo "unknown opt: $1" >&2; exit 2;;
esac; done
16. .claude/skills/demo-creator/scripts/build_artifact.sh (L34-L36)
[🟠 MEDIUM] os.path.normpath does not resolve relative paths to absolute ones. If base happens to be a relative path (e.g., if invoked dynamically differently than current shell expectation), p will also be relative. Comparing a relative p against the absolute path from os.path.abspath(base) via startswith() will always evaluate to False, failing to inline valid images. Consider using os.path.abspath to correctly resolve p.
Suggested change:
def datauri(rel):
p = os.path.abspath(os.path.join(base, rel))
if not (p.startswith(os.path.abspath(base)) and os.path.isfile(p)):
17. .claude/skills/demo-creator/scripts/build_artifact.sh (L51)
[🟠 MEDIUM] The bash script runs with set -e, but the embedded Python script exits with a 0 status code even when local assets cannot be found. Continuing the build process with missing assets will silently produce artifacts that break under strict CSP. Exiting with a non-zero status code here would correctly halt the build process.
Suggested change:
if missing:
print("ERROR: could not inline (will break under CSP): %s" % ", ".join(missing), file=sys.stderr)
sys.exit(1)
18. .claude/skills/demo-creator/scripts/build_artifact.sh (L55-L56)
[🔴 HIGH] The Chrome executable path is hardcoded to the macOS-specific application path. If this script is executed in Linux or Windows environments, the --render step will silently fail to find Chrome and skip rendering. It's recommended to add a cross-platform fallback to look up valid executables (e.g., google-chrome or chromium) in $PATH.
Suggested change:
CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"
if [ ! -x "$CHROME" ]; then
CHROME="$(command -v google-chrome || command -v chromium || echo "$CHROME")"
fi
SHOT="$DD/clips/frames/_artifact_render.png"; mkdir -p "$(dirname "$SHOT")"
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --fps) FPS="$2"; shift 2;; | ||
| --prefix) PREFIX="$2"; shift 2;; | ||
| *) echo "unknown opt: $1" >&2; exit 2;; | ||
| esac | ||
| done |
There was a problem hiding this comment.
[🟠 MEDIUM] Because set -euo pipefail (specifically set -u) is enabled, if the script is invoked with --fps or --prefix as the very last argument without providing a corresponding value, evaluating $2 will throw an unbound variable error and crash the script ungracefully (e.g., ./inspect.sh: line 13: $2: unbound variable).
You can add a check to explicitly ensure that the argument value is present, making the script more robust.
Suggested change:
| while [ $# -gt 0 ]; do | |
| case "$1" in | |
| --fps) FPS="$2"; shift 2;; | |
| --prefix) PREFIX="$2"; shift 2;; | |
| *) echo "unknown opt: $1" >&2; exit 2;; | |
| esac | |
| done | |
| while [ $# -gt 0 ]; do | |
| case "$1" in | |
| --fps) | |
| [ $# -ge 2 ] || { echo "--fps requires a value" >&2; exit 2; } | |
| FPS="$2"; shift 2;; | |
| --prefix) | |
| [ $# -ge 2 ] || { echo "--prefix requires a value" >&2; exit 2; } | |
| PREFIX="$2"; shift 2;; | |
| *) echo "unknown opt: $1" >&2; exit 2;; | |
| esac | |
| done |
|
|
||
| log "extracting frames @ ${FPS}fps -> $FRAMES" | ||
| ffmpeg -y -loglevel error -i "$GIF" -vf "fps=$FPS" "$FRAMES/f_%03d.png" | ||
| COUNT=$(ls "$FRAMES"/f_*.png 2>/dev/null | wc -l | tr -d ' ') |
There was a problem hiding this comment.
[🟠 MEDIUM] Due to set -euo pipefail, if ls fails to find any matching files (e.g., if ffmpeg produces no output due to an empty or corrupted GIF), it will exit with a non-zero status. This will cause the entire pipeline to fail and the script will silently abort at this line without printing any error message since stderr is redirected to /dev/null.
Additionally, using shell glob expansion with ls can result in an Argument list too long error if a massive number of frames is extracted.
Using find avoids pipeline failures when no files exist (since find safely returns 0) and protects against the argument list limit.
Suggested change:
| COUNT=$(ls "$FRAMES"/f_*.png 2>/dev/null | wc -l | tr -d ' ') | |
| COUNT=$(find "$FRAMES" -maxdepth 1 -name "f_*.png" 2>/dev/null | wc -l | tr -d ' ') |
| perl -pe 's/\x1b\[[0-9;]*[a-zA-Z]//g' "$SRC" > "$CLEAN" 2>/dev/null || cp "$SRC" "$CLEAN" | ||
| SRC="$CLEAN" |
There was a problem hiding this comment.
[🔵 LOW] Since SRC is reassigned to the temporary file $CLEAN, $(basename "$SRC") later in the script will output the temporary filename (e.g., tmp.xxxxxx) instead of the original log or JSON filename.
Consider storing the original filename before reassignment to preserve traceability.
Suggested change:
| perl -pe 's/\x1b\[[0-9;]*[a-zA-Z]//g' "$SRC" > "$CLEAN" 2>/dev/null || cp "$SRC" "$CLEAN" | |
| SRC="$CLEAN" | |
| perl -pe 's/\x1b\[[0-9;]*[a-zA-Z]//g' "$SRC" > "$CLEAN" 2>/dev/null || cp "$SRC" "$CLEAN" | |
| ORIG_SRC="$SRC" | |
| SRC="$CLEAN" |
| echo "# Learnings — $TOPIC / $ANGLE" | ||
| echo | ||
| echo "_Mined from \`$(basename "$SRC")\`$( [ -n "$TRACE_ID" ] && echo " · trace \`$TRACE_ID\`")._" |
There was a problem hiding this comment.
[🔵 LOW] Use the original source file name here to accurately report where the learnings were mined from.
Suggested change:
| echo "# Learnings — $TOPIC / $ANGLE" | |
| echo | |
| echo "_Mined from \`$(basename "$SRC")\`$( [ -n "$TRACE_ID" ] && echo " · trace \`$TRACE_ID\`")._" | |
| echo "# Learnings — $TOPIC / $ANGLE" | |
| echo | |
| echo "_Mined from \`$(basename "$ORIG_SRC")\`$( [ -n "$TRACE_ID" ] && echo " · trace \`$TRACE_ID\`")._" |
| extract_cmds() { # extract_cmds <file> | ||
| grep -oE '(\$ |bash |python |diff |altimate-code |duckdb |run_sql)[^"]*' "$1" 2>/dev/null \ | ||
| | sed -E 's/\\u[0-9a-fA-F]{4}//g' | grep -vE '^\s*$' | head -40 || true | ||
| } |
There was a problem hiding this comment.
[🟠 MEDIUM] The regex [^"]* will truncate the extracted command at the first double quote. While this correctly stops at the end of a JSON string property, it will prematurely truncate shell commands that themselves contain double quotes (e.g., python -c "print('test')" or escaped quotes like \").
Since this is explicitly a "best-effort miner", this might be an acceptable limitation, but if extracting complete command arguments is important, consider a more robust matching strategy or using a JSON parser like jq when SRC is a JSON file.
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | ||
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
There was a problem hiding this comment.
[🔴 HIGH] This array expansion will also trigger an "unbound variable" error under Bash 3.2 when the array is empty. Consider applying the same fallback syntax here.
Suggested change:
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | |
| "${M_ARGS[@]}" "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true | |
| ( cd "$CWD" && altimate-code run --yolo --max-turns "$MAXT" --format json --trace \ | |
| ${M_ARGS[@]+"${M_ARGS[@]}"} "$PROMPT" ) | tee "$RUNS/$ANGLE.json" || true |
| while [ $# -gt 0 ]; do case "$1" in | ||
| --out) OUT="$2"; shift 2;; | ||
| --render) RENDER=1; shift;; | ||
| --width) W="$2"; shift 2;; | ||
| --height) H="$2"; shift 2;; | ||
| *) echo "unknown opt: $1" >&2; exit 2;; | ||
| esac; done |
There was a problem hiding this comment.
[🟠 MEDIUM] Since set -u is enabled at the top of the script, referencing $2 without checking if it exists will throw an abrupt bash "unbound variable" error if an option like --out is passed as the very last argument. Additionally, shift 2 will fail if there are less than 2 arguments left. It's safer to explicitly check $# to provide a clear error message.
Suggested change:
| while [ $# -gt 0 ]; do case "$1" in | |
| --out) OUT="$2"; shift 2;; | |
| --render) RENDER=1; shift;; | |
| --width) W="$2"; shift 2;; | |
| --height) H="$2"; shift 2;; | |
| *) echo "unknown opt: $1" >&2; exit 2;; | |
| esac; done | |
| while [ $# -gt 0 ]; do case "$1" in | |
| --out) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; OUT="$2"; shift 2;; | |
| --render) RENDER=1; shift;; | |
| --width) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; W="$2"; shift 2;; | |
| --height) [ $# -ge 2 ] || { echo "missing value for $1" >&2; exit 2; }; H="$2"; shift 2;; | |
| *) echo "unknown opt: $1" >&2; exit 2;; | |
| esac; done |
| def datauri(rel): | ||
| p = os.path.normpath(os.path.join(base, rel)) | ||
| if not (p.startswith(os.path.abspath(base)) and os.path.isfile(p)): |
There was a problem hiding this comment.
[🟠 MEDIUM] os.path.normpath does not resolve relative paths to absolute ones. If base happens to be a relative path (e.g., if invoked dynamically differently than current shell expectation), p will also be relative. Comparing a relative p against the absolute path from os.path.abspath(base) via startswith() will always evaluate to False, failing to inline valid images. Consider using os.path.abspath to correctly resolve p.
Suggested change:
| def datauri(rel): | |
| p = os.path.normpath(os.path.join(base, rel)) | |
| if not (p.startswith(os.path.abspath(base)) and os.path.isfile(p)): | |
| def datauri(rel): | |
| p = os.path.abspath(os.path.join(base, rel)) | |
| if not (p.startswith(os.path.abspath(base)) and os.path.isfile(p)): |
| html = re.sub(r'(url\()(["\']?)([^)"\']+\.(?:gif|png|jpe?g|svg|webp))(["\']?\))', repl, html, flags=re.I) | ||
| open(out, "w", encoding="utf-8").write(html) | ||
| print("inlined %d asset(s); %.2f MB" % (inlined[0], len(html)/1e6)) | ||
| if missing: print("WARNING: could not inline (will break under CSP): %s" % ", ".join(missing), file=sys.stderr) |
There was a problem hiding this comment.
[🟠 MEDIUM] The bash script runs with set -e, but the embedded Python script exits with a 0 status code even when local assets cannot be found. Continuing the build process with missing assets will silently produce artifacts that break under strict CSP. Exiting with a non-zero status code here would correctly halt the build process.
Suggested change:
| if missing: print("WARNING: could not inline (will break under CSP): %s" % ", ".join(missing), file=sys.stderr) | |
| if missing: | |
| print("ERROR: could not inline (will break under CSP): %s" % ", ".join(missing), file=sys.stderr) | |
| sys.exit(1) |
| CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" | ||
| SHOT="$DD/clips/frames/_artifact_render.png"; mkdir -p "$(dirname "$SHOT")" |
There was a problem hiding this comment.
[🔴 HIGH] The Chrome executable path is hardcoded to the macOS-specific application path. If this script is executed in Linux or Windows environments, the --render step will silently fail to find Chrome and skip rendering. It's recommended to add a cross-platform fallback to look up valid executables (e.g., google-chrome or chromium) in $PATH.
Suggested change:
| CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" | |
| SHOT="$DD/clips/frames/_artifact_render.png"; mkdir -p "$(dirname "$SHOT")" | |
| CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" | |
| if [ ! -x "$CHROME" ]; then | |
| CHROME="$(command -v google-chrome || command -v chromium || echo "$CHROME")" | |
| fi | |
| SHOT="$DD/clips/frames/_artifact_render.png"; mkdir -p "$(dirname "$SHOT")" |
Harden the skill's scripts and docs per the PR #979 review (Codex, CodeRabbit, Kilo, cubic): - `lib.sh`: `demo_dir` now rejects non-slug topics (`..`, `/`) so paths can't escape `demos/`. - `baseline_vs_altimate.sh`: guard empty-array expansion for bash 3.2 (`${arr[@]+"${arr[@]}"}`); fix `OPENCODE_PERMISSION` to top-level tool keys (`{"bash":"deny"}`); warn loudly when `--reset-from` or `--model` is omitted (contaminated fixture / mismatched model invalidate the comparison); fail the script when the altimate run fails instead of `|| true`. - `record.sh`: add `asciinema --return` and propagate a failed recorded command so a broken run isn't rendered and passed off as a good clip. - `build_artifact.sh`: fix path-containment (realpath + `os.path.commonpath`, not string prefix); discover a Chrome/Chromium on PATH and hard-fail `--render` if none (render-inspection is mandatory). - `inspect.sh`: cross-check a `.baseline` clip against the baseline JSON, and count frames with `find` so a no-match doesn't trip `set -e`. - `setup_pyspark_fixture.sh`: honor an existing `JAVA_HOME`/`java`/`java_home` before Homebrew (works on Linux / Intel mac); robust major-version parse for `1.8`-style strings; validate prerequisites. - `extract_learnings.sh`: display the real source filename, not the temp copy. - `SKILL.md`: authenticity must come from the SAME recorded run (its `--trace`), not a second `--format json` run; add a language tag to the tree code block. - `blog.md.tmpl`: mark the baseline section comparison-only (showcase is default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the reviews. Pushed Correctness / safety
bash 3.2 (macOS default)
Comparison validity (the P1/P2 cluster)
Portability
Docs / template
A few low-value nitpicks (e.g. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.claude/skills/demo-creator/scripts/baseline_vs_altimate.sh (1)
78-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: declare/assign separately to avoid masking awk's exit status.
export OPENCODE_PERMISSION="{$(...)}"swallows a nonzero exit from theawkpipeline. Low risk here since it only builds a JSON string, but worth splitting per shellcheck SC2155.🤖 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/demo-creator/scripts/baseline_vs_altimate.sh at line 78, The OPENCODE_PERMISSION export in the baseline_vs_altimate.sh script is masking the exit status of the awk pipeline; split the declaration and assignment so failures from the JSON-building command are preserved. Update the shell logic around the OPENCODE_PERMISSION construction to assign the computed value first and then export it, keeping the behavior of the BASE_DENY/awk transformation unchanged.Source: Linters/SAST tools
.claude/skills/demo-creator/SKILL.md (1)
110-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded personal path reduces portability.
/Users/anandgupta/obsidian/altimate/Research/<topic>/<Title>.mdis a machine/user-specific absolute path baked into a shared skill definition. Anyone else running this skill (or the author on a different machine) will hit a copy step that fails or silently writes to the wrong location.Consider making the vault path configurable (env var or optional flag) with this as a documented default, rather than a hardcoded absolute path.
Also applies to: 204-205
🤖 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/demo-creator/SKILL.md around lines 110 - 112, The Obsidian vault copy path is hardcoded to a personal absolute location, which makes the demo-creator skill non-portable. Update the blog markdown copy step in the skill definition to read the vault destination from a configurable source such as an environment variable or optional flag, and keep the current path only as a documented default fallback. Ensure the global rule that writes to the Research/<topic>/<Title>.md location uses that configurable value consistently wherever the copy behavior is defined..claude/skills/demo-creator/scripts/record.sh (1)
31-34: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider
C.UTF-8overen_US.UTF-8for portability.
en_US.UTF-8isn't guaranteed to be generated on minimal/musl-based systems (e.g. Alpine, some CI containers), whereasC.UTF-8is broadly available and is already what the PTY layer defaults to (packages/opencode/src/pty/index.ts:162). If the requested locale doesn't exist, some tools print a "cannot set locale" warning to stderr — which risks leaking into the very recording this script is trying to keep clean.♻️ Suggested change
-export LC_ALL="${LC_ALL:-en_US.UTF-8}" LANG="${LANG:-en_US.UTF-8}" +export LC_ALL="${LC_ALL:-C.UTF-8}" LANG="${LANG:-C.UTF-8}"As per context snippet from
packages/opencode/src/pty/index.ts:162, the PTY environment locale is set to C.UTF-8 — aligning record.sh with this default improves consistency and portability.🤖 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/demo-creator/scripts/record.sh around lines 31 - 34, The locale fallback in record.sh is too specific and may fail on minimal or musl-based systems. Update the LC_ALL and LANG defaults in the recording setup to use C.UTF-8 instead of en_US.UTF-8, matching the PTY environment behavior in pty index and avoiding locale warnings during asciinema recording.
🤖 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.
Nitpick comments:
In @.claude/skills/demo-creator/scripts/baseline_vs_altimate.sh:
- Line 78: The OPENCODE_PERMISSION export in the baseline_vs_altimate.sh script
is masking the exit status of the awk pipeline; split the declaration and
assignment so failures from the JSON-building command are preserved. Update the
shell logic around the OPENCODE_PERMISSION construction to assign the computed
value first and then export it, keeping the behavior of the BASE_DENY/awk
transformation unchanged.
In @.claude/skills/demo-creator/scripts/record.sh:
- Around line 31-34: The locale fallback in record.sh is too specific and may
fail on minimal or musl-based systems. Update the LC_ALL and LANG defaults in
the recording setup to use C.UTF-8 instead of en_US.UTF-8, matching the PTY
environment behavior in pty index and avoiding locale warnings during asciinema
recording.
In @.claude/skills/demo-creator/SKILL.md:
- Around line 110-112: The Obsidian vault copy path is hardcoded to a personal
absolute location, which makes the demo-creator skill non-portable. Update the
blog markdown copy step in the skill definition to read the vault destination
from a configurable source such as an environment variable or optional flag, and
keep the current path only as a documented default fallback. Ensure the global
rule that writes to the Research/<topic>/<Title>.md location uses that
configurable value consistently wherever the copy behavior is defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 68c15b51-c7b7-48e0-a4c5-c187a39edc2b
📒 Files selected for processing (9)
.claude/skills/demo-creator/SKILL.md.claude/skills/demo-creator/scripts/baseline_vs_altimate.sh.claude/skills/demo-creator/scripts/build_artifact.sh.claude/skills/demo-creator/scripts/extract_learnings.sh.claude/skills/demo-creator/scripts/inspect.sh.claude/skills/demo-creator/scripts/lib.sh.claude/skills/demo-creator/scripts/record.sh.claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh.claude/skills/demo-creator/templates/blog.md.tmpl
🚧 Files skipped from review as they are similar to previous changes (6)
- .claude/skills/demo-creator/templates/blog.md.tmpl
- .claude/skills/demo-creator/scripts/lib.sh
- .claude/skills/demo-creator/scripts/extract_learnings.sh
- .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh
- .claude/skills/demo-creator/scripts/build_artifact.sh
- .claude/skills/demo-creator/scripts/inspect.sh
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".claude/skills/demo-creator/SKILL.md">
<violation number="1" location=".claude/skills/demo-creator/SKILL.md:159">
P2: Output layout requires `runs/<angle>.json` but Phase 5 changes remove the mechanism to produce it in showcase mode. Either update the output layout to note this file is comparison-mode-only or record it conditionally.</violation>
</file>
<file name=".claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh">
<violation number="1" location=".claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh:22">
P2: Do not accept Java 21 for the PySpark 3.5 fixture; Spark 3.5 is supported on Java 8/11/17, not 21. Remove `21` from the compatibility gate so the script keeps searching for a supported JDK.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| fallback (see `references/recording.md`). | ||
| - **Authenticity comes from the SAME recorded run, not a re-run.** The clip's proof is that | ||
| `.cast` PLUS the `--trace` written by *that* invocation (grab its trace id from the cast / | ||
| `altimate-code trace list`). Do NOT run `altimate-code run --format json` a second time to |
There was a problem hiding this comment.
P2: Output layout requires runs/<angle>.json but Phase 5 changes remove the mechanism to produce it in showcase mode. Either update the output layout to note this file is comparison-mode-only or record it conditionally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/demo-creator/SKILL.md, line 159:
<comment>Output layout requires `runs/<angle>.json` but Phase 5 changes remove the mechanism to produce it in showcase mode. Either update the output layout to note this file is comparison-mode-only or record it conditionally.</comment>
<file context>
@@ -152,14 +152,19 @@ advance until it passes.
+ fallback (see `references/recording.md`).
+- **Authenticity comes from the SAME recorded run, not a re-run.** The clip's proof is that
+ `.cast` PLUS the `--trace` written by *that* invocation (grab its trace id from the cast /
+ `altimate-code trace list`). Do NOT run `altimate-code run --format json` a second time to
+ "get the json" — a fresh agent run is nondeterministic and would authenticate a different
+ session than the one on screen. (In comparison mode, `baseline_vs_altimate.sh` captures
</file context>
| "$1/bin/java" -version 2>&1 | sed -nE 's/.*version "([0-9]+)(\.([0-9]+))?.*/\1 \3/p' | head -1 \ | ||
| | awk '{ if ($1==1) print $2; else print $1 }' | ||
| } | ||
| compat() { case "$1" in 8|11|17|21) return 0;; *) return 1;; esac; } |
There was a problem hiding this comment.
P2: Do not accept Java 21 for the PySpark 3.5 fixture; Spark 3.5 is supported on Java 8/11/17, not 21. Remove 21 from the compatibility gate so the script keeps searching for a supported JDK.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh, line 22:
<comment>Do not accept Java 21 for the PySpark 3.5 fixture; Spark 3.5 is supported on Java 8/11/17, not 21. Remove `21` from the compatibility gate so the script keeps searching for a supported JDK.</comment>
<file context>
@@ -9,27 +9,56 @@ FIXTURE="${1:?usage: setup_pyspark_fixture.sh <fixture-dir>}"
+ "$1/bin/java" -version 2>&1 | sed -nE 's/.*version "([0-9]+)(\.([0-9]+))?.*/\1 \3/p' | head -1 \
+ | awk '{ if ($1==1) print $2; else print $1 }'
+}
+compat() { case "$1" in 8|11|17|21) return 0;; *) return 1;; esac; }
+
+JAVA_HOME_FOUND=""
</file context>
| compat() { case "$1" in 8|11|17|21) return 0;; *) return 1;; esac; } | |
| compat() { case "$1" in 8|11|17) return 0;; *) return 1;; esac; } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13772ba13d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # It captures the json event stream for each run (authenticity proof). It does NOT render | ||
| # GIFs — use record.sh to capture the watchable clip of whichever run(s) you show. |
There was a problem hiding this comment.
Fresh evidence from this helper: it captures only the JSON streams and explicitly sends users to record.sh for the GIF/cast, so in comparison mode the clip being inspected comes from a separate agent invocation than runs/<angle>.json. For nondeterministic claude/altimate-code runs, inspect.sh can then authenticate a baseline or altimate clip against a different run, which breaks the skill's real-run evidence guarantee; record the cast/GIF and JSON from the same invocation or store a trace id from the recorded command.
Useful? React with 👍 / 👎.
| echo | ||
| echo "## Reusable lesson <!-- author: product bug / env gotcha / prompt that triggers the value / fixture pattern." | ||
| echo " If it should persist across sessions, also save to memory + MANIFEST ledger. -->" | ||
| } > "$OUT" |
There was a problem hiding this comment.
When the default series flow has 2–4 angles, every call to extract_learnings.sh <topic> <angle> writes to the same demos/<topic>/learnings.md with truncation, so mining the second clip deletes the commands and value evidence from the first. That leaves the blog/manifest grounded only in the last run instead of the whole series; append per-angle sections or write angle-specific learning files.
Useful? React with 👍 / 👎.
| html = re.sub(r'(url\()(["\']?)([^)"\']+\.(?:gif|png|jpe?g|svg|webp))(["\']?\))', repl, html, flags=re.I) | ||
| open(out, "w", encoding="utf-8").write(html) | ||
| print("inlined %d asset(s); %.2f MB" % (inlined[0], len(html)/1e6)) | ||
| if missing: print("WARNING: could not inline (will break under CSP): %s" % ", ".join(missing), file=sys.stderr) |
There was a problem hiding this comment.
When an HTML template has a mistyped or not-yet-rendered image path, datauri() records it in missing but this branch only prints a warning and the script still announces an artifact ready. The resulting page keeps a relative src/url() that Claude artifacts block under CSP, so a broken image can be published even though this helper is the self-contained gate; exit non-zero whenever missing is non-empty.
Useful? React with 👍 / 👎.
| "$1/bin/java" -version 2>&1 | sed -nE 's/.*version "([0-9]+)(\.([0-9]+))?.*/\1 \3/p' | head -1 \ | ||
| | awk '{ if ($1==1) print $2; else print $1 }' | ||
| } | ||
| compat() { case "$1" in 8|11|17|21) return 0;; *) return 1;; esac; } |
There was a problem hiding this comment.
On machines where the existing JAVA_HOME or java on PATH is JDK 21, this marks it compatible and stops searching, but the script then pins pyspark>=3.5,<4; the Spark 3.5 docs list Java 8/11/17 (https://spark.apache.org/docs/3.5.6/), not 21. Those users can fail the smoke test instead of falling through to an installed/Homebrew JDK 17, so restrict this compatibility list to the Spark 3.5-supported majors or upgrade the PySpark pin when choosing 21.
Useful? React with 👍 / 👎.
| if [ "$BASE_MODE" = "claude" ]; then | ||
| need claude | ||
| ( cd "$CWD" && claude -p --dangerously-skip-permissions --max-turns "$MAXT" \ | ||
| --output-format json --model "$BASE_MODEL" "$PROMPT" ) \ |
There was a problem hiding this comment.
For the claude baseline, this writes non-streaming --output-format json; the Claude CLI reference documents json as a print-mode output format and --verbose as the flag for full turn-by-turn output, with stream-json available for event streaming (https://docs.anthropic.com/en/docs/claude-code/cli-reference). In comparison mode runs/<angle>.baseline.json therefore lacks the tool-use/tool-result event stream that the script comment and inspection helpers treat as authenticity proof, so capture --output-format stream-json --verbose or another full transcript for the baseline.
Useful? React with 👍 / 👎.
| The blog markdown is ALSO copied to the Obsidian vault per the user's global rule: | ||
| `/Users/anandgupta/obsidian/altimate/Research/<topic>/<Title>.md`. |
There was a problem hiding this comment.
When this repo skill is run by anyone other than the user with this exact macOS home directory, phase 7 still instructs the agent to copy blog.md to /Users/anandgupta/.... On Linux, CI, or another developer's machine that path usually does not exist, so an otherwise completed demo can fail at delivery or write outside the project; make the vault destination optional/config-driven instead of a hard-coded absolute path.
Useful? React with 👍 / 👎.
| ## Reused skills (invoke, don't reimplement) | ||
| `deep-research` / `parallel:parallel-web-search` (phase 1), `blog-writer` + `viral-tech-blog` | ||
| (phase 7), `humanizer` + `consensus:review` (phase 8). |
There was a problem hiding this comment.
I searched this checkout for the referenced helper skills (deep-research, blog-writer, viral-tech-blog, humanizer, consensus:review) and none are included, so a user with only the repo contents can reach phases 1/7/8 and be told to invoke skills that do not exist. That blocks the advertised end-to-end demo workflow after setup/recording; either include these dependencies, document them as prerequisites, or provide self-contained fallbacks for the required phases.
Useful? React with 👍 / 👎.
dev-punia-altimate
left a comment
dev-punia-altimate
left a comment
There was a problem hiding this comment.
🤖 Code Review — OpenCodeReview (Gemini) — 4 finding(s)
- 4 anchored to a line (posted inline when the comment stream is on)
- 0 without a line anchor
All findings (full text)
1. .claude/skills/demo-creator/scripts/record.sh (L33)
[🟠 MEDIUM] The comment states "Force one", but the parameter expansion ${LC_ALL:-en_US.UTF-8} only provides a fallback when the variable is unset or empty. If a user already has a non-UTF-8 locale set (e.g., LC_ALL=C), this line will preserve that non-UTF-8 value, which may still cause the asciinema error you are trying to prevent.
To strictly enforce a UTF-8 locale, you should assign the values directly.
Suggested change:
export LC_ALL="en_US.UTF-8" LANG="en_US.UTF-8"
2. .claude/skills/demo-creator/scripts/inspect.sh (L29)
[🟠 MEDIUM] By default, find searches recursively and matches all file types (including directories). To make this strictly equivalent to the original ls "$FRAMES"/f_*.png command and avoid edge cases (like counting the directory itself if $PREFIX happens to match the f_*.png pattern, or unexpectedly counting files in subdirectories if any are created in the future), it is recommended to limit the search depth and restrict matches to regular files.
Suggested change:
COUNT=$(find "$FRAMES" -maxdepth 1 -type f -name 'f_*.png' 2>/dev/null | wc -l | tr -d ' ')
3. .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (L31-L38)
[🔴 HIGH] The PATH check occurs before the macOS /usr/libexec/java_home check. If java is found at /usr/bin/java (which acts as a stub on macOS and typically a symlink on Linux), dirname resolution assigns /usr to JAVA_HOME. This intercepts valid macOS setups, preventing them from resolving the true JDK directory, and assigns a directory without the standard JDK layout (like lib/), which breaks Spark and other JVM tools.
To fix this, we should prioritize the macOS java_home check over the PATH fallback. Additionally, resolving symlinks (via readlink or realpath) when computing JAVA_HOME from PATH is crucial to ensure it works correctly on Linux.
Suggested change:
# 1) caller's JAVA_HOME 2) macOS java_home for 17/11/8 3) java already on PATH
add_cand "${JAVA_HOME:-}"
if [ -z "$JAVA_HOME_FOUND" ] && [ -x /usr/libexec/java_home ]; then
for v in 17 11 21 8; do add_cand "$(/usr/libexec/java_home -v "$v" 2>/dev/null)"; done
fi
if [ -z "$JAVA_HOME_FOUND" ] && command -v java >/dev/null 2>&1; then
java_bin="$(command -v java)"
java_bin="$(readlink -f "$java_bin" 2>/dev/null || realpath "$java_bin" 2>/dev/null || echo "$java_bin")"
add_cand "$(cd "$(dirname "$java_bin")/.." && pwd)"
fi
4. .claude/skills/demo-creator/scripts/setup_pyspark_fixture.sh (L39-L44)
[🟠 MEDIUM] The fast-path check for Homebrew kegs hardcodes macOS Homebrew prefixes (/opt/homebrew/opt and /usr/local/opt) but omits the standard Linuxbrew prefix (/home/linuxbrew/.linuxbrew/opt). On Linux systems using Homebrew, this bypasses the fast path and unnecessarily invokes the much slower brew fallback.
Furthermore, Linuxbrew typically doesn't use the macOS-specific libexec/openjdk.jdk/Contents/Home layout, so adding the base keg path ($pfx/openjdk@$v) is required to properly detect the JDK on Linux platforms.
Suggested change:
# 4) Homebrew kegs (Apple Silicon + Intel + Linuxbrew prefixes)
if [ -z "$JAVA_HOME_FOUND" ]; then
for pfx in /opt/homebrew/opt /usr/local/opt /home/linuxbrew/.linuxbrew/opt; do
for v in 17 11 21; do
add_cand "$pfx/openjdk@$v/libexec/openjdk.jdk/Contents/Home"
add_cand "$pfx/openjdk@$v"
done
done
fi
|
|
||
| # asciinema (Rust) rejects args that aren't valid UTF-8 under a non-UTF-8 locale. Force one | ||
| # so prompts/banners with non-ASCII (em dashes, arrows, accents) don't error out. | ||
| export LC_ALL="${LC_ALL:-en_US.UTF-8}" LANG="${LANG:-en_US.UTF-8}" |
There was a problem hiding this comment.
[🟠 MEDIUM] The comment states "Force one", but the parameter expansion ${LC_ALL:-en_US.UTF-8} only provides a fallback when the variable is unset or empty. If a user already has a non-UTF-8 locale set (e.g., LC_ALL=C), this line will preserve that non-UTF-8 value, which may still cause the asciinema error you are trying to prevent.
To strictly enforce a UTF-8 locale, you should assign the values directly.
Suggested change:
| export LC_ALL="${LC_ALL:-en_US.UTF-8}" LANG="${LANG:-en_US.UTF-8}" | |
| export LC_ALL="en_US.UTF-8" LANG="en_US.UTF-8" |
| log "extracting frames @ ${FPS}fps -> $FRAMES" | ||
| ffmpeg -y -loglevel error -i "$GIF" -vf "fps=$FPS" "$FRAMES/f_%03d.png" | ||
| # Count without letting a no-match ls abort us under `set -e`/pipefail (lib.sh sets both). | ||
| COUNT=$(find "$FRAMES" -name 'f_*.png' 2>/dev/null | wc -l | tr -d ' ') |
There was a problem hiding this comment.
[🟠 MEDIUM] By default, find searches recursively and matches all file types (including directories). To make this strictly equivalent to the original ls "$FRAMES"/f_*.png command and avoid edge cases (like counting the directory itself if $PREFIX happens to match the f_*.png pattern, or unexpectedly counting files in subdirectories if any are created in the future), it is recommended to limit the search depth and restrict matches to regular files.
Suggested change:
| COUNT=$(find "$FRAMES" -name 'f_*.png' 2>/dev/null | wc -l | tr -d ' ') | |
| COUNT=$(find "$FRAMES" -maxdepth 1 -type f -name 'f_*.png' 2>/dev/null | wc -l | tr -d ' ') |
| # 1) caller's JAVA_HOME 2) java already on PATH 3) macOS java_home for 17/11/8 | ||
| add_cand "${JAVA_HOME:-}" | ||
| if [ -z "$JAVA_HOME_FOUND" ] && command -v java >/dev/null 2>&1; then | ||
| add_cand "$(cd "$(dirname "$(command -v java)")/.." && pwd)" | ||
| fi | ||
| if [ -z "$JAVA_HOME_FOUND" ] && [ -x /usr/libexec/java_home ]; then | ||
| for v in 17 11 21 8; do add_cand "$(/usr/libexec/java_home -v "$v" 2>/dev/null)"; done | ||
| fi |
There was a problem hiding this comment.
[🔴 HIGH] The PATH check occurs before the macOS /usr/libexec/java_home check. If java is found at /usr/bin/java (which acts as a stub on macOS and typically a symlink on Linux), dirname resolution assigns /usr to JAVA_HOME. This intercepts valid macOS setups, preventing them from resolving the true JDK directory, and assigns a directory without the standard JDK layout (like lib/), which breaks Spark and other JVM tools.
To fix this, we should prioritize the macOS java_home check over the PATH fallback. Additionally, resolving symlinks (via readlink or realpath) when computing JAVA_HOME from PATH is crucial to ensure it works correctly on Linux.
Suggested change:
| # 1) caller's JAVA_HOME 2) java already on PATH 3) macOS java_home for 17/11/8 | |
| add_cand "${JAVA_HOME:-}" | |
| if [ -z "$JAVA_HOME_FOUND" ] && command -v java >/dev/null 2>&1; then | |
| add_cand "$(cd "$(dirname "$(command -v java)")/.." && pwd)" | |
| fi | |
| if [ -z "$JAVA_HOME_FOUND" ] && [ -x /usr/libexec/java_home ]; then | |
| for v in 17 11 21 8; do add_cand "$(/usr/libexec/java_home -v "$v" 2>/dev/null)"; done | |
| fi | |
| # 1) caller's JAVA_HOME 2) macOS java_home for 17/11/8 3) java already on PATH | |
| add_cand "${JAVA_HOME:-}" | |
| if [ -z "$JAVA_HOME_FOUND" ] && [ -x /usr/libexec/java_home ]; then | |
| for v in 17 11 21 8; do add_cand "$(/usr/libexec/java_home -v "$v" 2>/dev/null)"; done | |
| fi | |
| if [ -z "$JAVA_HOME_FOUND" ] && command -v java >/dev/null 2>&1; then | |
| java_bin="$(command -v java)" | |
| java_bin="$(readlink -f "$java_bin" 2>/dev/null || realpath "$java_bin" 2>/dev/null || echo "$java_bin")" | |
| add_cand "$(cd "$(dirname "$java_bin")/.." && pwd)" | |
| fi |
| # 4) Homebrew kegs (Apple Silicon + Intel prefixes) | ||
| if [ -z "$JAVA_HOME_FOUND" ]; then | ||
| for pfx in /opt/homebrew/opt /usr/local/opt; do | ||
| for v in 17 11 21; do add_cand "$pfx/openjdk@$v/libexec/openjdk.jdk/Contents/Home"; done | ||
| done | ||
| fi |
There was a problem hiding this comment.
[🟠 MEDIUM] The fast-path check for Homebrew kegs hardcodes macOS Homebrew prefixes (/opt/homebrew/opt and /usr/local/opt) but omits the standard Linuxbrew prefix (/home/linuxbrew/.linuxbrew/opt). On Linux systems using Homebrew, this bypasses the fast path and unnecessarily invokes the much slower brew fallback.
Furthermore, Linuxbrew typically doesn't use the macOS-specific libexec/openjdk.jdk/Contents/Home layout, so adding the base keg path ($pfx/openjdk@$v) is required to properly detect the JDK on Linux platforms.
Suggested change:
| # 4) Homebrew kegs (Apple Silicon + Intel prefixes) | |
| if [ -z "$JAVA_HOME_FOUND" ]; then | |
| for pfx in /opt/homebrew/opt /usr/local/opt; do | |
| for v in 17 11 21; do add_cand "$pfx/openjdk@$v/libexec/openjdk.jdk/Contents/Home"; done | |
| done | |
| fi | |
| # 4) Homebrew kegs (Apple Silicon + Intel + Linuxbrew prefixes) | |
| if [ -z "$JAVA_HOME_FOUND" ]; then | |
| for pfx in /opt/homebrew/opt /usr/local/opt /home/linuxbrew/.linuxbrew/opt; do | |
| for v in 17 11 21; do | |
| add_cand "$pfx/openjdk@$v/libexec/openjdk.jdk/Contents/Home" | |
| add_cand "$pfx/openjdk@$v" | |
| done | |
| done | |
| fi |
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
2 participants
What
Adds
.claude/skills/demo-creator/— a skill that turns a one-line problem statementinto a REAL recorded terminal demo + a blog showing how to do it with
altimate-codeandwhy that's worth it.
Pipeline
deep-research the real pain → critique → design → record a genuine
altimate-code run→extract what the session teaches → visually inspect the frames (and the published page) →
write the blog → critique loop, then publish as a Claude artifact.
Design notes
showcasemode (default) showsaltimate-codealone and letsthe specific thing it does carry the value;
comparisonmode adds aclaude -pcontrolonly when the contrast is the point and
altimate-codeactually wins.--format json(no faking); value is shown, not asserted.
extract_learnings.shmines the session intolearnings.md(commands run, value-moment evidence, reusable lessons) — feeding the blog and a
cross-session lesson ledger.
build_artifact.shinlines clips as base64 andrenders headless so the page is visually inspected before publishing.
blog-writer,viral-tech-blog, andhumanizerskills.Validation
Dogfooded end-to-end on a PySpark SQL-port demo (
claude-codevsaltimate-code): bothtranslate correctly, but
altimate-coderuns the reference SQL anddiffs its output toprove equivalence while
claude-codeonly asserts it. Demo artifacts are intentionallykept out of this PR — this PR is the skill only.
Contents
SKILL.md,references/(value-wedge, recording, quality-bar),scripts/(record, inspect, baseline_vs_altimate, setup_pyspark_fixture, extract_learnings,
build_artifact, lib),
templates/(blog, replicate, vhs tape).🤖 Generated with Claude Code
Summary by cubic
Adds
demo-creator, a skill that turns a one‑line brief into a real recorded terminal demo and a blog showing how to do it withaltimate-codeand why it matters. This update also hardens scripts and docs to enforce fair comparisons and fail‑safe authenticity.New Features
showcase(default) and optionalcomparisonvsclaude-code; only compare whenaltimate-codeactually wins.record.sh(asciinema+agg) andinspect.shfor frames and trace cross‑check; optionalvhsfallback.extract_learnings.sh→learnings.md;build_artifact.shinlines media and headless‑renders before publishing; templates and a PySpark fixture helper.Bug Fixes
lib.shenforces slug‑only topics;build_artifact.shusesrealpath/commonpath, discovers Chrome/Chromium, and hard‑fails--renderif missing.baseline_vs_altimate.shwarns on missing--reset-from/--model, fixesOPENCODE_PERMISSION, guards Bash 3.2 array expansion, and fails when thealtimate-coderun fails.record.shpropagates command exits viaasciinema --return;inspect.shcounts frames robustly and checks.baselineclips against the baseline JSON.setup_pyspark_fixture.shdetects existing JDKs (8/11/17/21) across macOS/Linux before Homebrew, validates prerequisites, and smoke‑tests Spark.SKILL.mdclarifies same‑run trace requirement;blog.md.tmpltags the baseline section as comparison‑only;extract_learnings.shshows the real source filename.Written for commit 13772ba. Summary will update on new commits.
Summary by CodeRabbit