Fix _GatedLLM.completion override after _return_metrics removal (rel-1.29.0)

[VascoSch92]

Problem

Pre-commit (pyright) fails on rel-1.29.0 (blocking the v1.29.0 release PR #3787) with two errors in tests/agent_server/test_goal_loop.py:

test_goal_loop.py:42 - error: Method "completion" overrides class "TestLLM" in an incompatible manner
test_goal_loop.py:58 - error: Expected 4 positional arguments (reportCallIssue)

This is a logical merge conflict between two changes that are each green on their own:

• 2bedbd8f "Remove acp_env and _return_metrics deprecations (removed_in 1.29.0)" — deleted the _return_metrics parameter from LLM.completion / TestLLM.completion (now 4 positional params). This lives on this release branch.
• feat(agent-server): add /goal agent-server endpoint, background loop, and stop/resume #3770 "add /goal agent-server endpoint…" — added test_goal_loop.py, whose _GatedLLM.completion was written against the old signature (with _return_metrics) and CI'd before the removal landed.

The release branch is the first tree where pyright type-checks both together.

Note: this targets rel-1.29.0, not main. The _return_metrics removal is only on the release branch — on main the parameter still exists, so this override is correct there and must not be changed.

Fix

Drop _return_metrics from the _GatedLLM.completion override — both the signature and the super().completion(...) call — to match the 4-positional signature on this branch. No behavior change.

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
TOTAL	32765	14163	56%

report-only-changed-files is enabled. No files were changed during this commit :)

[all-hands-bot]

✅ QA Report: PASS

The PR fixes the _GatedLLM.completion override mismatch on rel-1.29.0: the baseline raises the reported positional-argument failure, while the PR delegates past the signature boundary successfully.

Does this PR achieve its stated goal?

Yes. The stated goal is to remove the stale _return_metrics parameter from _GatedLLM.completion on the rel-1.29.0 branch so it matches TestLLM.completion after the release-branch deprecation removal. Runtime introspection shows the PR branch signature now matches the base class shape, and executing the affected completion path no longer raises the baseline TypeError from forwarding one positional argument too many.

Phase	Result
Environment Setup	✅ make build completed successfully and installed the uv environment
CI Status	⚠️ Functional CI is green including pre-commit; Validate PR description is failing and QA checks are in progress
Functional Verification	✅ Before/after execution confirms the override mismatch is fixed

Functional Verification

Test 1: Runtime signature alignment

Step 1 — Reproduce / establish baseline without the fix:
Ran:

git checkout --detach origin/rel-1.29.0
PYTHONPATH="$PWD" OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_goal_loop_signature.py

Relevant output:

Base TestLLM.completion: (self, messages: 'list[Message]', tools: 'Sequence[ToolDefinition] | None' = None, add_security_risk_prediction: 'bool' = False, on_token: 'TokenCallbackType | None' = None, **kwargs: 'Any') -> 'LLMResponse'
_GatedLLM.completion: (self, messages, tools=None, _return_metrics=False, add_security_risk_prediction=False, on_token=None, **kwargs)

This shows the baseline mismatch: the base release-branch method has no _return_metrics, while _GatedLLM still exposes it.

Step 2 — Apply the PR's changes:
Checked out fix/goal-loop-test-return-metrics-rel-1.29.0 at ad1251175418a038ea4828ea616ca63d47df1212.

Step 3 — Re-run with the fix in place:
Ran:

PYTHONPATH="$PWD" OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_goal_loop_signature.py

Relevant output:

Base TestLLM.completion: (self, messages: 'list[Message]', tools: 'Sequence[ToolDefinition] | None' = None, add_security_risk_prediction: 'bool' = False, on_token: 'TokenCallbackType | None' = None, **kwargs: 'Any') -> 'LLMResponse'
_GatedLLM.completion: (self, messages, tools=None, add_security_risk_prediction=False, on_token=None, **kwargs)

This shows the stale _return_metrics parameter is gone from the override.

Test 2: Affected completion call path

Step 1 — Reproduce / establish baseline without the fix:
Ran:

git checkout --detach origin/rel-1.29.0
PYTHONPATH="$PWD" OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_goal_loop_completion.py

Output:

TypeError
TestLLM.completion() takes from 2 to 5 positional arguments but 6 were given

This confirms the baseline bug at runtime too: _GatedLLM.completion forwards the removed _return_metrics argument into TestLLM.completion, producing the same kind of signature mismatch the PR describes.

Step 2 — Apply the PR's changes:
Checked out fix/goal-loop-test-return-metrics-rel-1.29.0 at ad1251175418a038ea4828ea616ca63d47df1212.

Step 3 — Re-run with the fix in place:
Ran:

PYTHONPATH="$PWD" OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_goal_loop_completion.py

Output:

TestLLMExhaustedError
TestLLM: no more scripted responses (exhausted after 0 calls)

This is the expected next behavior for the minimal harness with no scripted TestLLM responses. The important part is that the positional-argument TypeError is gone, proving the changed override now delegates through the current release-branch signature.

Issues Found

None.

This review was created by an AI agent (OpenHands) on behalf of the user.