[#2580] Optimised .vortex/CLAUDE.md and unified the snapshot update path.#2583
Open
AlexSkrypnyk wants to merge 2 commits into
Open
[#2580] Optimised .vortex/CLAUDE.md and unified the snapshot update path.#2583AlexSkrypnyk wants to merge 2 commits into
AlexSkrypnyk wants to merge 2 commits into
Conversation
…ath. Removed the Bash-commands rule block (kept in AGENTS.md), consolidated the snapshot HARD RULES, and made 'ahoy update-snapshots' the only documented path by dropping the 'UPDATE_SNAPSHOTS' advert from the env table and the installer test docblock. Moved the documentation-video pipeline internals to 'docs/CLAUDE.md', fixed the wrong working directory in 'tests/CLAUDE.md', and deduplicated the shell-script pattern.
WalkthroughThis PR reorganizes and standardizes the maintenance documentation across the vortex repository's CLAUDE.md guides. It removes outdated bash-command rules, restructures guidance for snapshots and videos, standardizes the snapshot regeneration workflow to use ChangesDocumentation and Workflow Standardization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.vortex/docs/CLAUDE.md:
- Around line 80-81: The code currently calls $recorder->fail("Docker stack
'$compose_project' is not running") and returns 1 when needs_built_project is
true and isDockerStackRunning($compose_project) is false, which contradicts the
docs for --keep; change the behavior so it exits cleanly instead of failing:
replace the fail(...) + return 1 with a non-error path (e.g., call
$recorder->note(...) or $recorder->info(...) to log the message and return 0),
keeping the existing note('Rerun without --keep to bootstrap fresh.') and
ensuring this branch returns success rather than an error code.
- Around line 94-104: Update the example sequence in the CLAUDE.md docs to
remove the incorrect first use of the --keep flag: the initial command should be
"ahoy update-videos lint" to perform the full bootstrap + record lint (since
--keep skips bootstrap and requires an existing workspace), then show "ahoy
update-videos --keep lint" to reuse the kept workspace, and finally "ahoy
update-videos lint" for a fresh bootstrap that cleans up; adjust the three
example lines around the ahoy update-videos invocation accordingly so comments
match behavior.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 2043d645-0e39-4567-9039-10d03398e8d3
📒 Files selected for processing (4)
.vortex/CLAUDE.md.vortex/docs/CLAUDE.md.vortex/installer/tests/Functional/Handlers/AbstractHandlerProcessTestCase.php.vortex/tests/CLAUDE.md
Comment on lines
+80
to
+81
| bootstraps from scratch; `--keep` reuses the existing workspace and exits cleanly | ||
| if the Docker stack is not running. |
There was a problem hiding this comment.
Correct the exit behavior for missing Docker stack.
The documentation states that --keep "exits cleanly if the Docker stack is not running," but the orchestrator actually fails with exit code 1 and error message when the stack is missing.
Context snippet 4 shows:
if ($needs_built_project && !$recorder->isDockerStackRunning($compose_project)) {
$recorder->fail("Docker stack '$compose_project' is not running");
$recorder->note('Rerun without --keep to bootstrap fresh.');
return 1;
}📝 Proposed fix
-bootstraps from scratch; `--keep` reuses the existing workspace and exits cleanly
-if the Docker stack is not running.
+bootstraps from scratch; `--keep` reuses the existing workspace and fails if the
+Docker stack is not running (when recording commands that need Docker).🤖 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 @.vortex/docs/CLAUDE.md around lines 80 - 81, The code currently calls
$recorder->fail("Docker stack '$compose_project' is not running") and returns 1
when needs_built_project is true and isDockerStackRunning($compose_project) is
false, which contradicts the docs for --keep; change the behavior so it exits
cleanly instead of failing: replace the fail(...) + return 1 with a non-error
path (e.g., call $recorder->note(...) or $recorder->info(...) to log the message
and return 0), keeping the existing note('Rerun without --keep to bootstrap
fresh.') and ensuring this branch returns success rather than an error code.
Comment on lines
+94
to
+104
| **Iterating on one video** - use `--keep` so the install + build happens only | ||
| once, then replay the recording against the preserved project: | ||
|
|
||
| ```bash | ||
| cd .vortex | ||
| ahoy update-videos --keep lint # full bootstrap + record lint, keep workspace | ||
| # tweak the lint command or the recorder | ||
| ahoy update-videos --keep lint # reuse the kept workspace, record lint only | ||
| ahoy update-videos lint # fresh bootstrap, no --keep, cleans up at end | ||
| # (or manually) rm -rf .artifacts/tmp/videos-workspace | ||
| ``` |
There was a problem hiding this comment.
Fix the iteration workflow example - first command cannot use --keep.
The workflow example shows ahoy update-videos --keep lint as the first command with a comment claiming "full bootstrap + record lint, keep workspace". This is incorrect:
--keepskips the bootstrap and requires an existing workspace (lines 80-81 and the command matrix at line 74).- If no workspace exists,
--keepwill fail immediately (verified by snippet 4's workspace existence check). - The comment "full bootstrap" contradicts the
--keepflag's documented behavior.
🔧 Proposed fix
**Iterating on one video** - use `--keep` so the install + build happens only
once, then replay the recording against the preserved project:
```bash
cd .vortex
-ahoy update-videos --keep lint # full bootstrap + record lint, keep workspace
+ahoy update-videos lint # full bootstrap + record lint, keep workspace
# tweak the lint command or the recorder
ahoy update-videos --keep lint # reuse the kept workspace, record lint only
-ahoy update-videos lint # fresh bootstrap, no --keep, cleans up at end
# (or manually) rm -rf .artifacts/tmp/videos-workspace</details>
<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 @.vortex/docs/CLAUDE.md around lines 94 - 104, Update the example sequence in
the CLAUDE.md docs to remove the incorrect first use of the --keep flag: the
initial command should be "ahoy update-videos lint" to perform the full
bootstrap + record lint (since --keep skips bootstrap and requires an existing
workspace), then show "ahoy update-videos --keep lint" to reuse the kept
workspace, and finally "ahoy update-videos lint" for a fresh bootstrap that
cleans up; adjust the three example lines around the ahoy update-videos
invocation accordingly so comments match behavior.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- cr-comment:v1:7793e7f6588848c32895ab95 -->
<!-- This is an auto-generated comment by CodeRabbit -->
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
added
the
Needs review
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2583 +/- ##
=======================================
Coverage 88.38% 88.38%
=======================================
Files 7 7
Lines 353 353
Branches 3 3
=======================================
Hits 312 312
Misses 41 41 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2580
Summary
The
.vortex/CLAUDE.mdmaintenance guide had grown to 282 lines, carrying content duplicated from the always-loaded rootAGENTS.mdand the sibling subsystem guides, four near-identicalupdate-snapshotsrule blocks, and a deep video-pipeline reference that only matters when editing the video system itself. This pass trims it to 174 lines (~38% smaller), removes the duplication, verifies the remaining instructions against the currentahoytargets and code, and makesahoy update-snapshotsthe single documented way to regenerate fixtures.Changes
.vortex/CLAUDE.mdAGENTS.md, so it no longer lands in context twice.update-snapshotsHARD RULE blocks into a singleSnapshotssection that preserves every constraint (commit first, foreground only, no wrappers, theahoy --filepointer).ahoy update-snapshotsthe only documented path: dropped theUPDATE_SNAPSHOTSrow from the Environment Variables table and added an explicit instruction never to setUPDATE_SNAPSHOTSby hand.VIDEOSconfig array,--keepiteration) out to.vortex/docs/CLAUDE.md, keeping a concise staleness-trigger summary plus a pointer..vortex/docs/CLAUDE.mdDocumentation videossection holding the pipeline internals relocated from the maintenance guide, where they sit next to theupdate-videos.phporchestrator they describe..vortex/tests/CLAUDE.mdCommandsblock, which pointed at.vortex/installerwith installer commands instead of.vortex/tests..vortex/CLAUDE.md..vortex/installer/tests/Functional/Handlers/AbstractHandlerProcessTestCase.phpUPDATE_SNAPSHOTS=1" to "Runahoy update-snapshotsfrom.vortex/", removing the last place that advertised the second snapshot path.Before / After
Summary by CodeRabbit
Documentation
Chores