Skip to content

docs: update engine/tool command docs for the JSON argv rendering pipeline - #657

Merged
k-rister merged 2 commits into
masterfrom
docs-rickshaw-argv-rendering
Aug 14, 2026
Merged

docs: update engine/tool command docs for the JSON argv rendering pipeline#657
k-rister merged 2 commits into
masterfrom
docs-rickshaw-argv-rendering

Conversation

@k-rister

Copy link
Copy Markdown
Contributor

Summary

  • docs/how-engines-work.md and docs/how-tool-collection-works.md both still described the removed bash-array command mechanism (declare -a ARGS=(...) && cmd "${ARGS[@]}") and the old "command" JSON key.
  • Following rickshaw#868 (merged), benchmark and tool commands are shipped as JSON argv lists rather than pre-rendered shell strings, with shlex.join() applied exactly once, immediately before each execution. Tools now use the same --arg=value grammar as benchmarks (previously '--arg' 'val' bash-array tokens).

Test plan

  • Diffed against rickshaw#868's actual merged code to confirm accuracy (file/JSON shapes, shlex.join() timing, tool/benchmark grammar convergence).

…eline

Both docs still described the bash-array command mechanism and the
"command" JSON key removed by rickshaw#868. Benchmark and tool commands
are now shipped as JSON argv lists (shlex.join()'d immediately before
execution, not pre-rendered into a shell string), with tools using the
same --arg=value grammar as benchmarks.
@k-rister k-rister self-assigned this Aug 14, 2026
@k-rister
k-rister requested a review from a team August 14, 2026 21:07
@project-crucible-tracking project-crucible-tracking Bot moved this to In Progress in Crucible Tracking Aug 14, 2026
@k-rister

k-rister commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

PR Review: crucible#657 — docs: update engine/tool command docs for the JSON argv rendering pipeline

Summary: Updates engine and tool collection documentation to reflect the migration from pre-rendered shell strings and Bash array command formats (declare -a ARGS) to JSON argv lists parsed and joined using shlex.join() immediately before execution.
Changed files: 2
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style, Completeness

Documentation

  • [docs/how-engines-work.md:412] Outdated command file delivery mechanism — Describes Command files as using "(the ARGS array mechanism)", which was deprecated and removed in this PR. It should be updated to refer to the argv list mechanism.
  • [docs/how-tool-collection-works.md:74] Outdated tool parameter format — States that parameters become --key value argument pairs, which is stale. It should be updated to --key=value (or --arg=value) format to match the new argv serialization convention.

File Coverage

  • docs/how-engines-work.md — 1 doc issue
  • docs/how-tool-collection-works.md — 1 doc issue

Missing from diff:

  • docs/implementing-a-new-tool.md — This file is missing from the diff but must be updated because it serves as the developer guide for writing new tools. It contains multiple outdated references to the --key value argument pair format:
    • [Line 316]: They receive tool parameters as command-line arguments in --key value format.
    • [Line 352]: 3. Parse --key value arguments with defaults
    • [Line 576]: Each parameter becomes a --key value argument pair passed to the start script.
      These should be updated to --key=value format to maintain consistency with the benchmark implementation guide and match the converged grammar.

Limitations

  • Runtime verification: The review is limited to static analysis of the documentation text and cross-referencing against the rickshaw repository configuration code. No runtime verification or container-level execution tests of the command JSON deserialization process were performed.

Verdict

Approve with comments — The documentation changes accurately capture the structural and behavioral shifts introduced by the argv list rendering pipeline. However, several legacy references to the old ARGS array and --key value argument passing formats were missed in the changed files and the tool developer implementation guide. These should be resolved before merging.


Review compiled and generated by Antigravity (AI Coding Assistant)
Powered by Gemini 1.5 Pro — Advanced Agentic Coding Team, Google DeepMind

Two additional prose references to the old formats survived the first
pass: how-engines-work.md's "Data delivery mechanisms" summary still
named the removed ARGS array, and how-tool-collection-works.md's
earlier tool-params overview still described --key value (space-
separated) instead of --key=value. Also updates
implementing-a-new-tool.md, which wasn't touched in the original PR
despite documenting the same --key value convention in three places.
@k-rister

Copy link
Copy Markdown
Contributor Author

Fixed in e2df31e — verified all three findings directly:

  • docs/how-engines-work.md:412 — "Data delivery mechanisms" summary section still named the removed ARGS array (missed in the first pass since it's prose, not literal declare -a syntax). Now says "the JSON argv list mechanism".
  • docs/how-tool-collection-works.md:74 — an earlier tool-params overview section (separate from the one already updated) still said --key value. Fixed to --key=value.
  • docs/implementing-a-new-tool.md — correct that this file was missing from the diff entirely. Updated all three references (lines 316, 352, 576) to --key=value.

Also swept all crucible docs for any other "ARGS array"/"key value"-style phrasing and confirmed none remain.

@atheurer atheurer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The documentation updates are clear, highly accurate, and successfully clean up all legacy Bash array references.

@k-rister
k-rister merged commit c49e64a into master Aug 14, 2026
16 checks passed
@k-rister
k-rister deleted the docs-rickshaw-argv-rendering branch August 14, 2026 21:49
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Crucible Tracking Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants