OpenHands / OpenHands/software-agent-sdk

CriticMixin always calls evaluate() with git_patch=None, so critics grade the transcript instead of the diff

Open
#4,554 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug evaluation priority:medium sdk
Dominant language
Python
Stars
1.1k
Forks
539
Avg merge
1d 19h
Merged PRs (30d)
137

Description

Summary

CriticBase.evaluate() is declared as

def evaluate(self, events: Sequence["LLMConvertibleEvent"], git_patch: str | None = None) -> CriticResult

but the only in-SDK caller, CriticMixin._evaluate_with_critic, always passes git_patch=None:

https://github.com/OpenHands/software-agent-sdk/blob/main/openhands-sdk/openhands/sdk/agent/critic_mixin.py#L64-L65

# Evaluate without git_patch for now
result = self.critic.evaluate(events=events, git_patch=None)

So the git_patch parameter is unreachable in normal operation, and any critic that wants to judge what the agent actually changed has to fall back to reading the conversation transcript.

Why this matters in practice

We run a custom CriticBase subclass that scores whether an agent's work genuinely satisfied its task. With git_patch=None it grades the transcript, and that produces false negatives that depend on which tools the agent happened to use:

  • Agent uses terminal with cat/heredoc, so file contents appear in the transcript, and the critic scores correctly.
  • Agent uses file_editor for the identical, correct change, so contents never enter the transcript, the critic sees no evidence the work was done, and scores it 0.0.

Measured on agent-server 1.42.1: the same correct task scored 0.000 on the first pass and only reached 1.000 after an IterativeRefinementConfig retry. That is a wasted refinement loop and a wasted LLM call on work that was already right.

EmptyPatchCritic and AgentFinishedCritic do not hit this because neither looks at content, and APIBasedCritic targets a separate classification service, so the gap is invisible unless you implement your own critic.

Suggested fix

Compute the patch in _evaluate_with_critic and pass it through when the conversation workspace is inside a git work tree. openhands.sdk.git already exposes the read helpers (git_changes, git_diff), so most of the plumbing exists.

Two edge cases worth handling, both of which we hit:

  1. No HEAD. LocalWorkspace git-inits a fresh workspace and never commits, so git diff --cached HEAD exits 128 and yields nothing. Diffing against the empty tree (git hash-object -t tree /dev/null) works there.
  2. Do not mutate the user index. Staging in order to compute a diff will clobber a real index. Pointing GIT_INDEX_FILE at a throwaway file avoids touching it.

A pre-existing dirty working tree also means a naive diff includes changes the agent never made, so scoping the patch to the agent's own edits, or at least documenting that caveat, would help.

Alternative

If passing the patch by default is unwanted for performance reasons, making it opt-in via a field on CriticBase would still be better than a parameter no caller can reach.

Happy to open a PR if the approach above sounds right.

Environment

  • openhands-sdk / openhands-agent-server 1.42.1
  • Custom CriticBase subclass loaded via --extra-python-path / --import-modules

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read openhands-sdk/openhands/sdk/agent/critic_mixin.py, especially _evaluate_with_critic, then inspect the read helpers in openhands.sdk.git. Verify how patches behave for a fresh repository without HEAD and with an existing user index. Done means CriticBase.evaluate() receives the relevant agent patch without mutating the user's index.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, python
Domain
ai, devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.