On Sat, 25 Jul 2026 16:43:45 +0800
[email protected] wrote:
> From: Chengwen Feng <[email protected]>
>
> Currently review-patch.py only supports cloud AI providers
> (Anthropic, OpenAI, xAI, Google) via REST API, requiring API keys.
>
> Add a --via option that invokes the locally installed opencode CLI as
> the review runner instead of making HTTP calls. opencode reads
> AGENTS.md from the DPDK project directory automatically, needing no
> configuration beyond opencode on PATH.
>
> The --via and -p/--provider options are independent -- via routes to
> the local agent mode while -p continues to use the cloud API path.
>
> Signed-off-by: Chengwen Feng <[email protected]>
>
> ---
Good idea, AI review with Claude Opus found:
Errors:
1. opencode "error" events are silently dropped.
The parse loop handles only "text" and "step_finish". The event
stream also emits {"type":"error","error":{"name":...,
"data":{"message":...}}} for rate limits and provider failures.
If a session errors after emitting some text, the partial review
is returned as if complete, and classify_review() will exit 0.
A review tool reporting "clean" on an aborted run is the worst
failure mode here. Handle it:
elif event_type == "error":
err = event.get("error", {})
error(f"opencode: {err.get('name', '')}: "
f"{err.get('data', {}).get('message', '')}")
2. All text parts are concatenated, including intermediate narration.
Unlike a single-shot API call, an agent interleaves text with tool
calls, so text_parts collects the model's running commentary
("Let me read AGENTS.md first...") along with the actual review.
That commentary ends up in the output file, in the --send-email
body, and is scanned by classify_review() -- a narration line
starting with "Error" flips the exit code to 3.
Keep only the final assistant message: track part.messageID and
emit the parts whose messageID matches the step_finish that has
reason == "stop".
Warnings:
3. agents_path is passed to --file unresolved.
patch_temp is absolute (tempfile), but str(agents_path) is
whatever the user gave and defaults to the relative "AGENTS.md",
while opencode is launched with --dir pointing at the repo root
rather than the caller's cwd. Use str(agents_path.resolve()).
Related: the commit message says opencode picks up AGENTS.md from
the project directory automatically. If so, --file agents_path is
redundant in the default case, and with -a it adds a second file
rather than replacing the project one -- so -a does not mean what
it means on the cloud path. Pick one behaviour and state it.
4. Verbose logs are captured and discarded.
-v appends --print-logs, which writes to stderr, but
capture_output=True swallows stderr and it is only surfaced
(truncated to 500 chars) on non-zero exit. So --via -v gives the
user none of the logs it just asked for. Either drop
--print-logs or pass stderr=None when verbose.
5. Failure diagnostics are thrown away.
JSONDecodeError does "continue", so if stdout is ever not JSONL
(schema change, older opencode, a wrapper printing a banner)
every line is skipped and the user gets only "No review text
received from opencode". Include the first few hundred chars of
stdout in that error message.
6. Options silently ignored in --via mode.
-p/--provider, --auth, -t/--tokens and --max-tokens have no
effect; only --large-file warns. Since -p has a default you
cannot tell "unset" from "explicitly set" -- give it
default=None and resolve to anthropic later, then parser.error()
when --via is combined with an explicit -p or --auth.
The commit message's claim that "--via and -p/--provider are
independent -- ... -p continues to use the cloud API path" is not
what the code does: --via wins and -p is dropped.
7. Documentation not updated in the same patch.
doc/guides/contributing/patches.rst, "AI-Assisted Patch Review",
states the script supports four providers and that an API key must
be set. --via opencode is user-visible and removes that
requirement. Code and docs go in the same commit.
8. The agent runs with the default toolset in the source tree.
opencode run --dir <dpdk root> with no --agent or permission
restriction gives the reviewing agent write/edit/bash on the
working tree. A review should not be able to modify the tree it
is reviewing. Restrict it to a read/grep/glob agent, or document
the exposure.
9. black reformats the new code in four places: the cmd list literal,
the argument packing in _run_review(), and lines 1211 and 1269
(93 chars each). Line 1287 is pre-existing but this patch
re-indents it, so it may as well be wrapped now.
Info:
10. estimated_tokens = 1 / max_input_tokens = 0 as a sentinel so that
"if estimated_tokens > 0" means "not already reviewed" is
fragile. An explicit already_reviewed flag reads better.
11. usage.api_calls = 1 if steps > 0 else 0 -- an agent run is
"steps" calls, not one, and print_token_summary() suppresses the
entire summary when api_calls == 0, so a run with text but no
step_finish prints nothing under --show-tokens. Use
usage.api_calls = steps.
12. Metadata is inconsistent: provider_name is "OpenCode" while the
JSON "provider" field gets args.via, i.e. "opencode".
13. compare-patch-reviews.sh only iterates providers that have API
keys and has no way to include the local runner. Worth a
follow-up if comparing local against cloud is the point.
