[#2351] Added per-step timing to 'task()' and 'pass()' functions.#2473
Merged
AlexSkrypnyk merged 11 commits intomainfrom Apr 23, 2026
Merged
[#2351] Added per-step timing to 'task()' and 'pass()' functions.#2473AlexSkrypnyk merged 11 commits intomainfrom
AlexSkrypnyk merged 11 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds per-task timing to many shell scripts: Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/lint.scripts.sh:
- Around line 20-35: mask_protected() currently masks content between "#
`@formatter`:off" and "# `@formatter`:on" but silently continues if a closing marker
is missing, reducing shfmt coverage; modify the awk logic in mask_protected() to
detect an unclosed in_block by tracking in_block and, after processing the file,
if in_block is still true print a descriptive error to stderr (including the
file name) and exit non‑zero so the script fails fast; keep the existing
behavior of printing masked lines (using indent and ":"), but ensure the awk or
surrounding shell checks the exit status and returns a failing code when
in_block remains open.
- Line 64: The pipeline calling mask_protected in the lint step can hide
failures because pipefail isn't enabled; enable strict pipe failure handling by
setting 'set -o pipefail' (or 'set -euo pipefail') before the pipeline or
rewrite the check to ensure mask_protected's exit status is propagated (e.g.,
run the pipeline inside a group with pipefail), so the conditional around the
pipeline (the line invoking mask_protected "${file}" | LC_ALL=C.UTF-8 shfmt -i 2
-ci -s -d) fails when mask_protected fails.
In `@scripts/vortex/deploy-container-registry.sh`:
- Around line 39-41: The timing logic shows only the last task because
_TASK_START is set by each task() call but pass() is invoked only once at the
end; fix by pairing each task() call with a corresponding pass() immediately
after that task's work completes (replace the single final pass() usage with
per-step pass() calls). Locate the task() invocations and the lone pass() at the
end (see functions task() and pass()) and move/add pass() so every task() has a
matching pass() to capture and print that task's elapsed time.
In `@scripts/vortex/deploy-lagoon.sh`:
- Around line 85-87: The pass() helper can carry a previous task's _TASK_START
into the final summary, so change pass() to accept an optional second flag (e.g.
"no_duration") and if that flag is set skip computing the duration; always unset
_TASK_START before returning to avoid leaking timers; then update the final
summary call to invoke pass with that "no_duration" flag. Specifically: update
the pass() function to check $2 (or an env var) and if not set compute and print
the duration and unset _TASK_START, but if set skip the duration calculation and
still unset _TASK_START; change the final summary invocation to call pass
"Summary message" "no_duration".
In `@scripts/vortex/export-db-image.sh`:
- Around line 33-35: The per-step timing is wrong because task() unconditionally
resets _TASK_START on every call so the final pass() only shows the last task
duration; change task() to only set _TASK_START when it is not already set
(e.g., only initialize _TASK_START if unset) and keep pass() behavior of
computing the elapsed time and unsetting _TASK_START after printing; this
preserves the start time across multiple task() calls and allows pass() to
report the full duration for the grouped steps (refer to the task() and pass()
functions in the diff).
In `@scripts/vortex/reset.sh`:
- Around line 16-18: The issue is that task() overwrites the global _TASK_START
so pass() only reports the last segment's duration; change the implementation to
maintain a stack/array of start timestamps (e.g., _TASK_START_STACK) when task()
is called (push current epoch and optional task name) and modify pass() to pop
the most recent start, compute and report that segment's elapsed time without
discarding previous entries so each task/pass pair reports its own duration;
update task(), pass() and any places referencing _TASK_START accordingly so
timing is per-step and stack-safe.
🪄 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: e89643b7-06d7-45a5-b4ff-8263daaa32ee
📒 Files selected for processing (39)
.vortex/tests/lint.scripts.shscripts/custom/provision-20-migration.shscripts/vortex/deploy-artifact.shscripts/vortex/deploy-container-registry.shscripts/vortex/deploy-lagoon.shscripts/vortex/deploy-webhook.shscripts/vortex/deploy.shscripts/vortex/doctor.shscripts/vortex/download-db-acquia.shscripts/vortex/download-db-container-registry.shscripts/vortex/download-db-ftp.shscripts/vortex/download-db-lagoon.shscripts/vortex/download-db-s3.shscripts/vortex/download-db-url.shscripts/vortex/download-db.shscripts/vortex/export-db-file.shscripts/vortex/export-db-image.shscripts/vortex/export-db.shscripts/vortex/info.shscripts/vortex/login-container-registry.shscripts/vortex/login.shscripts/vortex/logout.shscripts/vortex/notify-email.shscripts/vortex/notify-github.shscripts/vortex/notify-jira.shscripts/vortex/notify-newrelic.shscripts/vortex/notify-slack.shscripts/vortex/notify-webhook.shscripts/vortex/notify.shscripts/vortex/provision-sanitize-db.shscripts/vortex/provision.shscripts/vortex/reset.shscripts/vortex/setup-ssh.shscripts/vortex/task-copy-db-acquia.shscripts/vortex/task-copy-files-acquia.shscripts/vortex/task-custom-lagoon.shscripts/vortex/task-purge-cache-acquia.shscripts/vortex/update-vortex.shscripts/vortex/upload-db-s3.sh
Comment on lines
+39
to
+41
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| info() { [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[36m[INFO] %s\033[0m\n" "${1}" || printf "[INFO] %s\n" "${1}"; } | ||
| pass() { [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s\033[0m\n" "${1}" || printf "[ OK ] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
Pair task() and pass() per step; current timing only reflects the last step.
_TASK_START is reset at each task() call, but pass() is called once at the end. The elapsed suffix on Line 104 therefore reports only the final task duration, not per-step timing.
Suggested patch
@@
task "Processing service ${service}."
# Check if the service is running.
cid=$(docker compose ps -q "${service}")
@@
[ -z "${cid}" ] && fail "Service \"${service}\" is not running." && exit 1
note "Found \"${service}\" service container with id \"${cid}\"."
+ pass "Processed service ${service}."
@@
task "Committing container image with name \"${new_image}\"."
iid=$(docker commit "${cid}" "${new_image}")
iid="${iid#sha256:}"
note "Committed container image with id \"${iid}\"."
+ pass "Committed container image ${new_image}."
@@
task "Pushing container image to the registry."
docker push "${new_image}"
+ pass "Pushed container image ${new_image}."
done
-pass "Finished container registry deployment."
+info "Finished container registry deployment."Also applies to: 85-86, 95-96, 100-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/vortex/deploy-container-registry.sh` around lines 39 - 41, The timing
logic shows only the last task because _TASK_START is set by each task() call
but pass() is invoked only once at the end; fix by pairing each task() call with
a corresponding pass() immediately after that task's work completes (replace the
single final pass() usage with per-step pass() calls). Locate the task()
invocations and the lone pass() at the end (see functions task() and pass()) and
move/add pass() so every task() has a matching pass() to capture and print that
task's elapsed time.
Member
Author
There was a problem hiding this comment.
By design. The helper spec is: task() sets _TASK_START (overwriting any prior value); pass() reads the elapsed time since the most recent task(), prints the (Xs) suffix, and unsets _TASK_START. So the (Xs) on the final pass "Finished container registry deployment." is the duration of the most recent task() (the push), which is the documented behaviour. Restructuring this script's task/pass pairing is a separate concern outside the scope of this PR.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — thank you for the clarification. The task()/pass() contract is intentionally a "last-wins" model: each task() overwrites _TASK_START, and pass() always measures from the most recent task() call. The final pass "Finished container registry deployment." therefore correctly reports the push step's duration, not a cumulative total. I'll withdraw the suggestion.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2473
File: scripts/vortex/deploy-container-registry.sh:40-41
Timestamp: 2026-04-23T03:21:43.730Z
Learning: In the Vortex project (drevops/vortex), the task()/pass() timing design is intentional: task() always overwrites _TASK_START with the current epoch time, and pass() always measures elapsed time since the most recent task() call, printing the (Xs) suffix and unsetting _TASK_START. A lone pass() at the end of a script correctly reports the duration of the last task() invocation. Restructuring task/pass pairing for per-step timing is a separate concern and not required by the current design.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
Comment on lines
+85
to
+87
| task() { _TASK_START=$(date +%s); [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[34m[TASK] %s\033[0m\n" "${1}" || printf "[TASK] %s\n" "${1}"; } | ||
| info() { [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[36m[INFO] %s\033[0m\n" "${1}" || printf "[INFO] %s\n" "${1}"; } | ||
| pass() { [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s\033[0m\n" "${1}" || printf "[ OK ] %s\n" "${1}"; } | ||
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; [ "${TERM:-}" != "dumb" ] && tput colors >/dev/null 2>&1 && printf "\033[32m[ OK ] %s%s\033[0m\n" "${1}" "${_d}" || printf "[ OK ] %s%s\n" "${1}" "${_d}"; } |
There was a problem hiding this comment.
Prevent stale task timing from leaking into the final summary pass.
Line 87 appends timing whenever _TASK_START exists. In this script, the final summary at Line 275 can inherit the last task’s timer and report a misleading duration.
Suggested minimal fix
if [ "${exit_code}" = "0" ]; then
+ unset _TASK_START
pass "Finished LAGOON deployment."
else
fail "LAGOON deployment completed with errors."
fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/vortex/deploy-lagoon.sh` around lines 85 - 87, The pass() helper can
carry a previous task's _TASK_START into the final summary, so change pass() to
accept an optional second flag (e.g. "no_duration") and if that flag is set skip
computing the duration; always unset _TASK_START before returning to avoid
leaking timers; then update the final summary call to invoke pass with that
"no_duration" flag. Specifically: update the pass() function to check $2 (or an
env var) and if not set compute and print the duration and unset _TASK_START,
but if set skip the duration calculation and still unset _TASK_START; change the
final summary invocation to call pass "Summary message" "no_duration".
Member
Author
There was a problem hiding this comment.
By design. pass() already unsets _TASK_START after reading it; if no preceding task() set the timer, no suffix is shown. In this script the final summary pass() reports the duration of the most recent preceding task() per the helper spec. Restructuring the script's summary semantics is a separate concern outside the scope of this PR.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — the guard [ -n "${_TASK_START:-}" ] already prevents any suffix when no preceding task() has run, and the final summary intentionally inherits the last task's timer per the helper spec. I'll withdraw the concern.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2473
File: scripts/vortex/deploy-lagoon.sh:86-87
Timestamp: 2026-04-23T03:21:47.053Z
Learning: In the Vortex project (drevops/vortex), the `pass()` helper function is intentionally designed to show the elapsed time from the most recent `task()` call, including on final summary `pass()` calls. The `_TASK_START` variable is already guarded with `[ -n "${_TASK_START:-}" ]` so no suffix appears if no preceding `task()` set the timer. Showing the duration on the final summary `pass()` (e.g., "Finished LAGOON deployment.") is by design, not a timing leak, and should not be flagged as an issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1714
File: scripts/vortex/doctor.sh:69-70
Timestamp: 2025-06-03T03:15:29.849Z
Learning: In the Vortex project, logging functions (task(), info(), note(), pass(), fail(), warn()) are intentionally duplicated across all script files to make each script self-contained and independent, rather than sharing them through a common library. This design choice prioritizes script independence over reducing code duplication.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2473 +/- ##
==========================================
- Coverage 79.93% 79.51% -0.43%
==========================================
Files 129 122 -7
Lines 6883 6736 -147
Branches 47 3 -44
==========================================
- Hits 5502 5356 -146
+ Misses 1381 1380 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/custom/provision-10-example.sh`:
- Around line 53-54: Replace the unconditional success message after the
tolerated-failure install: stop using "drush pm:install redis || true" followed
immediately by pass "Installed Redis module." and instead run the install
without swallowing errors, check its exit status, and only call pass "Installed
Redis module." when the install command succeeds; if it fails, log or call a
different helper (e.g., a failure/warn path) so the script does not report
success while the "drush pm:install redis" command actually failed.
🪄 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: d953da5d-2203-4dc5-a5de-35d7d8220415
⛔ Files ignored due to path filters (10)
.vortex/installer/tests/Fixtures/handler_process/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (2)
scripts/custom/provision-10-example.shscripts/custom/provision-20-migration.sh
Comment on lines
53
to
+54
| drush pm:install redis || true | ||
| pass "Installed Redis module." |
There was a problem hiding this comment.
Do not report Redis install as successful when failure is explicitly tolerated.
With drush pm:install redis || true, the next line always prints success, even when install fails. This produces misleading operational output.
Suggested fix
- drush pm:install redis || true
- pass "Installed Redis module."
+ if drush pm:install redis; then
+ pass "Installed Redis module."
+ else
+ note "Redis module was not installed. Continuing."
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| drush pm:install redis || true | |
| pass "Installed Redis module." | |
| if drush pm:install redis; then | |
| pass "Installed Redis module." | |
| else | |
| note "Redis module was not installed. Continuing." | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/custom/provision-10-example.sh` around lines 53 - 54, Replace the
unconditional success message after the tolerated-failure install: stop using
"drush pm:install redis || true" followed immediately by pass "Installed Redis
module." and instead run the install without swallowing errors, check its exit
status, and only call pass "Installed Redis module." when the install command
succeeds; if it fails, log or call a different helper (e.g., a failure/warn
path) so the script does not report success while the "drush pm:install redis"
command actually failed.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/custom/provision-10-example.sh (1)
50-51:⚠️ Potential issue | 🟠 MajorDo not print success when Redis install failure is explicitly tolerated.
drush pm:install redis || trueallows failure, but the next line always reports success, which can mislead operators.Suggested fix
- drush pm:install redis || true - pass "Installed Redis module." + if drush pm:install redis; then + pass "Installed Redis module." + else + note "Redis module was not installed. Continuing." + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/custom/provision-10-example.sh` around lines 50 - 51, The script currently runs the tolerated command `drush pm:install redis || true` but unconditionally calls `pass "Installed Redis module."`, which misreports success; update the logic around the `drush pm:install redis || true` invocation to capture the command exit status and only call the `pass` helper when the exit code is zero (and otherwise call a `warn`/`fail`/log helper or echo a non-fatal warning), or remove the `|| true` and handle success/failure explicitly so `pass` accurately reflects a successful install; look for the `drush pm:install redis || true` invocation and the `pass` call to modify the flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/custom/provision-10-example.sh`:
- Around line 50-51: The script currently runs the tolerated command `drush
pm:install redis || true` but unconditionally calls `pass "Installed Redis
module."`, which misreports success; update the logic around the `drush
pm:install redis || true` invocation to capture the command exit status and only
call the `pass` helper when the exit code is zero (and otherwise call a
`warn`/`fail`/log helper or echo a non-fatal warning), or remove the `|| true`
and handle success/failure explicitly so `pass` accurately reflects a successful
install; look for the `drush pm:install redis || true` invocation and the `pass`
call to modify the flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c92fd292-0bc5-4fde-82e7-c58a42a25e14
⛔ Files ignored due to path filters (34)
.vortex/installer/tests/Fixtures/handler_process/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_base/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_demo/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_search/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_search_without_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_admin_toolbar/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_coffee/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_config_split/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_config_update/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_environment_indicator/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_pathauto/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_redirect/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_robotstxt/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_seckit_shield_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_shield/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_file/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_string/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_clamav/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_redis/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (1)
scripts/custom/provision-10-example.sh
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/2351-step-timing
branch
from
fb9d617 to
13ec3db
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
scripts/custom/provision-10-example.sh (1)
50-51:⚠️ Potential issue | 🟠 MajorAvoid unconditional success after a tolerated Redis install failure.
Line [50] swallows install failure, but Line [51] always reports success. That makes the output inaccurate.
Suggested fix
- drush pm:install redis || true - pass "Installed Redis module." + if drush pm:install redis; then + pass "Installed Redis module." + else + note "Redis module was not installed. Continuing." + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/custom/provision-10-example.sh` around lines 50 - 51, The script currently swallows Redis install failures by using "drush pm:install redis || true" and then unconditionally calling pass "Installed Redis module."; change this to run "drush pm:install redis" without "|| true" and check its exit status (e.g., if drush ...; then pass "Installed Redis module."; else log an error or call a fail/notify function with the error and do not claim success) so the script only reports success when the install actually succeeded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/custom/provision-20-migration.sh`:
- Around line 42-47: Add a missing fail() helper to match the other logging
helpers (info(), note(), task(), pass()) so error branches that call fail()
don't error with "command not found"; implement fail() to printf a clear error
message (e.g., " ==> %s\n" or similar formatting consistent with
info()/note()) and then exit with a non-zero status (exit 1) so the script stops
on failures invoked by fail().
- Around line 155-157: The script currently suppresses failures by appending "||
true" to the drush migrate:rollback --all command and then always calls pass
"Rolled back all migrations.", which masks errors; change the block around the
drush migrate:rollback call (the task "Rolling back all migrations." section) to
remove "|| true", capture the command's exit status, and only call pass "Rolled
back all migrations." when the exit code is zero—otherwise log an error and exit
non‑zero (or call your existing fail/abort helper) so failures are reported
instead of silenced.
- Around line 45-46: The task() and pass() helpers should record and report
elapsed time like vortex scripts: modify task() to set a start timestamp
variable (use _TASK_START) when called and print the existing task line, and
modify pass() to compute the elapsed seconds from _TASK_START (safe-guard if
unset), append the elapsed time in the "(Ns)" format to the success message,
then unset _TASK_START; update the functions named task() and pass() accordingly
so each invocation measures that step's duration.
---
Duplicate comments:
In `@scripts/custom/provision-10-example.sh`:
- Around line 50-51: The script currently swallows Redis install failures by
using "drush pm:install redis || true" and then unconditionally calling pass
"Installed Redis module."; change this to run "drush pm:install redis" without
"|| true" and check its exit status (e.g., if drush ...; then pass "Installed
Redis module."; else log an error or call a fail/notify function with the error
and do not claim success) so the script only reports success when the install
actually succeeded.
🪄 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: 10eb8472-0e55-45e3-b462-50d5b751b617
⛔ Files ignored due to path filters (34)
.vortex/installer/tests/Fixtures/handler_process/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_base/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_demo/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_no_search/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/custom_modules_search_without_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_acquia/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_container_registry/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_ftp/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_s3/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/scripts/custom/provision-20-migration.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_admin_toolbar/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_coffee/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_config_split/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_config_update/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_environment_indicator/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_pathauto/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_redirect/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_robotstxt/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_seckit_shield_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_shield/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_no_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_file/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_string/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_clamav/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_redis/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_no_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/services_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (2)
scripts/custom/provision-10-example.shscripts/custom/provision-20-migration.sh
Comment on lines
+45
to
+46
| task() { printf " > %s\n" "${1}"; } | ||
| pass() { printf " < %s\n" "${1}"; } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check for timing primitives in helper functions:"
rg -n '_TASK_START|date \+%s|^\s*task\(\)|^\s*pass\(\)' scripts/custom/provision-20-migration.shRepository: drevops/vortex
Length of output: 192
🏁 Script executed:
head -50 scripts/custom/provision-20-migration.sh | cat -nRepository: drevops/vortex
Length of output: 2083
🏁 Script executed:
grep -n "task\|pass" scripts/custom/provision-20-migration.sh | head -30Repository: drevops/vortex
Length of output: 789
🏁 Script executed:
# Check other custom scripts to see if they implement task/pass with timing
find scripts/custom -name "*.sh" -type f | head -10Repository: drevops/vortex
Length of output: 137
🏁 Script executed:
grep -A 3 "^task()\|^pass()" scripts/custom/provision-10-example.shRepository: drevops/vortex
Length of output: 179
🏁 Script executed:
grep -n "provision-20-migration\|task\|pass" scripts/provision.sh | head -20Repository: drevops/vortex
Length of output: 111
🏁 Script executed:
find . -name "provision.sh" -type f | grep -v node_modulesRepository: drevops/vortex
Length of output: 317
🏁 Script executed:
grep -A 5 -B 5 "provision-20-migration" scripts/vortex/provision.shRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
grep -n "custom\|provision" scripts/vortex/provision.sh | head -20Repository: drevops/vortex
Length of output: 1120
🏁 Script executed:
grep -n "VORTEX_PROVISION_SCRIPTS_DIR\|source\|\./" scripts/vortex/provision.sh | grep -A 3 -B 3 VORTEX_PROVISION_SCRIPTS_DIRRepository: drevops/vortex
Length of output: 864
🏁 Script executed:
sed -n '420,430p' scripts/vortex/provision.shRepository: drevops/vortex
Length of output: 510
🏁 Script executed:
grep -B 5 -A 5 "^task()\|^pass()" scripts/vortex/provision.sh | head -20Repository: drevops/vortex
Length of output: 1037
🏁 Script executed:
# Check if there are any tests for custom provision scripts
find . -path "*/tests/*" -name "*custom*" -o -path "*/tests/*" -name "*provision-20*" | grep -v node_modulesRepository: drevops/vortex
Length of output: 10952
🏁 Script executed:
head -50 "./.vortex/installer/tests/Fixtures/handler_process/migration_enabled/scripts/custom/provision-20-migration.sh" | tail -20Repository: drevops/vortex
Length of output: 728
Implement elapsed-time tracking in custom script's task()/pass() helpers for consistency with vortex pattern.
The custom script defines its own local task() and pass() helpers without timing support. While they function correctly for output formatting, they deviate from the vortex convention where task() sets _TASK_START and pass() computes and displays the elapsed duration (Xs).
Implementing timing here would enable performance tracking for individual steps (migrations, database operations, module enabling) within the script, consistent with how vortex scripts operate.
Proposed fix
-task() { printf " > %s\n" "${1}"; }
-pass() { printf " < %s\n" "${1}"; }
+task() { _TASK_START=$(date +%s); printf " > %s\n" "${1}"; }
+pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; printf " < %s%s\n" "${1}" "${_d}"; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| task() { printf " > %s\n" "${1}"; } | |
| pass() { printf " < %s\n" "${1}"; } | |
| task() { _TASK_START=$(date +%s); printf " > %s\n" "${1}"; } | |
| pass() { _d=""; [ -n "${_TASK_START:-}" ] && _d=" ($(($(date +%s) - _TASK_START))s)" && unset _TASK_START; printf " < %s%s\n" "${1}" "${_d}"; } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/custom/provision-20-migration.sh` around lines 45 - 46, The task()
and pass() helpers should record and report elapsed time like vortex scripts:
modify task() to set a start timestamp variable (use _TASK_START) when called
and print the existing task line, and modify pass() to compute the elapsed
seconds from _TASK_START (safe-guard if unset), append the elapsed time in the
"(Ns)" format to the success message, then unset _TASK_START; update the
functions named task() and pass() accordingly so each invocation measures that
step's duration.
Comment on lines
155
to
+157
| task "Rolling back all migrations." | ||
| drush migrate:rollback --all || true | ||
| pass "Rolled back all migrations." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'migrate:rollback --all \|\| true|pass "Rolled back all migrations\."' scripts/custom/provision-20-migration.shRepository: drevops/vortex
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Show full context around the problematic lines
echo "=== Full context around lines 155-157 ==="
sed -n '145,165p' scripts/custom/provision-20-migration.sh
echo ""
echo "=== Check for other || true patterns in the file ==="
rg -n '\|\| true' scripts/custom/provision-20-migration.sh
echo ""
echo "=== Check how task/pass/note functions are defined ==="
rg -n 'function (task|pass|note)|^(task|pass|note)\(\)' scripts/custom/provision-20-migration.sh
echo ""
echo "=== Search for similar error handling patterns ==="
rg -n 'if.*; then' scripts/custom/provision-20-migration.sh | head -20Repository: drevops/vortex
Length of output: 1777
False success reporting when rollback fails.
Line 156 suppresses the drush migrate:rollback --all exit code with || true, but line 157 unconditionally prints a success message. This masks rollback failures and reports success even when the command failed, creating false confidence in migration state.
Proposed fix
- drush migrate:rollback --all || true
- pass "Rolled back all migrations."
+ if drush migrate:rollback --all; then
+ pass "Rolled back all migrations."
+ else
+ note "Rollback returned non-zero; continuing as configured."
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/custom/provision-20-migration.sh` around lines 155 - 157, The script
currently suppresses failures by appending "|| true" to the drush
migrate:rollback --all command and then always calls pass "Rolled back all
migrations.", which masks errors; change the block around the drush
migrate:rollback call (the task "Rolling back all migrations." section) to
remove "|| true", capture the command's exit status, and only call pass "Rolled
back all migrations." when the exit code is zero—otherwise log an error and exit
non‑zero (or call your existing fail/abort helper) so failures are reported
instead of silenced.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
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 #2351
Summary
task()/pass()helper functions across all 38 Vortex scripts so operators can see how long each provision step took.# @formatter:off/# @formatter:onawareness tolint.scripts.shsoshfmthonors existing IDE-formatter guard markers and stops expanding the intentionally single-line helper functions.Changes
Commit 1 - Per-step timing in
task()andpass()(38 files)Scope:
scripts/vortex/*.sh(37 files) andscripts/custom/provision-20-migration.sh.task()now captures_TASK_START=$(date +%s)at call time.pass()reads_TASK_START, computes elapsed seconds, appends(Xs)to its output, then unsets the variable. If_TASK_STARTis unset (e.g. an unpairedpass()call used for skip messages), no suffix is shown.Before:
After:
Notes:
scripts/custom/provision-10-example.shwas intentionally skipped - it uses a simplifiedtask()format with no matchingpass()call._TASK_STARTand_duse leading-underscore names to avoid collisions with script-level variables.(0s), which is acceptable given whole-second precision.provision.sh(start_time) is independent and untouched.assert_output_contains "Cache was rebuilt."), so the appended(Xs)does not break them.Commit 2 - Honor
@formatter:offmarkers inlint.scripts.sh(1 file)Added a
mask_protected()awk helper in.vortex/tests/lint.scripts.shthat replaces lines between# @formatter:offand# @formatter:onwith indent-preserved:no-op placeholders before passing the file toshfmt -d. This ensuresshfmtrespects the same IDE-formatter guard markers already used in the codebase, preventing it from expanding the intentionally single-line helper functions into multi-line form.Output comparison
Before:
After:
Required snapshot update
9 installer fixture files are stale and will cause PHPUnit installer tests to fail until regenerated.
Affected files are under:
These fixtures are regenerated automatically from the root template files and must not be edited directly. Per the installer guide, run the following from the
.vortex/directory after merging:This is a known follow-up step (takes 10-15 minutes) and does not block review of the logic changes in this PR.
Summary by CodeRabbit
New Features
Bug Fixes
Chores