Skip to content

fix: show LLM provenance in automation output - #547

Open
enyst wants to merge 1 commit into
mainfrom
openhands/show-llm-provenance
Open

fix: show LLM provenance in automation output#547
enyst wants to merge 1 commit into
mainfrom
openhands/show-llm-provenance

Conversation

@enyst

@enyst enyst commented Sep 9, 2026

Copy link
Copy Markdown
Member

HUMAN:
This PR proposes a fix to show the LLM in automations. Which LLM is running can be relevant, and it is for us specially with PR review, as they don’t all have the same behavior.

  • A human has tested these changes.

Why

PR-review and Slack automation results do not identify the LLM profile and concrete model that generated them, making model-dependent judgments difficult to calibrate. The output implementations live in OpenHands/extensions, while the linked issue was filed in the Automation service repository.

Summary

  • Resolve the automation's selected AUTOMATION_MODEL profile through the Agent Server and use that concrete LLM for spawned reviewer and Slack conversations.
  • Append the same concise footer everywhere: LLM profile: <profile> · Model: <model>.
  • Cover GitHub review instructions/fallbacks and Slack completion output with regression tests; bump and regenerate the reviewer bundle to 1.0.1.

Issue Number

Tracks OpenHands/automation#430

Fixes #548

How to Test

uv sync --group test
npm run build:automations
uv run pytest -q
uv run python scripts/sync_extensions.py --check
uv run python -m py_compile skills/github-pr-reviewer/scripts/main.py skills/slack-channel-monitor/scripts/main.py
git diff --check

Expected result: 802 passed, 24 skipped. The sync check emits one pre-existing, non-blocking coverage warning for plugins/issue-duplicate-checker.

Video/Screenshots

Not applicable; this changes generated GitHub/Slack text output and includes automated request-body assertions.

Notes

  • The linked issue was classified as enhancement and marked ready-for-dev after confirming it already contained the required Desired Behavior and Acceptance Criteria sections.
  • Successful GitHub reviews are posted by the review agent, so the exact footer is required in its deterministic publishing prompt. Script-owned fallback and error comments append it directly.

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

@enyst can click here to continue refining the PR

Co-authored-by: openhands <openhands@all-hands.dev>
@all-hands-bot

Copy link
Copy Markdown
Contributor

🤖 OpenHands is reviewing this PR.

Head commit: 7a0dba57a6454c4163f71b8e3711405238fb6b84
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/b5176e74-a8ae-4f4f-a5b9-70d0bb4c5986

This comment was posted by an AI agent (OpenHands).

@all-hands-bot

Copy link
Copy Markdown
Contributor

🤖 OpenHands is reviewing this PR.

Head commit: 7a0dba57a6454c4163f71b8e3711405238fb6b84
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/023d9f90-1538-4503-8e98-2af81e1ac368

This comment was posted by an AI agent (OpenHands).

@all-hands-bot

Copy link
Copy Markdown
Contributor

⚠️ OpenHands PR Reviewer encountered a problem at commit 7a0dba57a645 (status: error).

This comment was posted by an AI agent (OpenHands).

@all-hands-bot
all-hands-bot removed their request for review September 9, 2026 23:52
@enyst

enyst commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@OpenHands /codereview this pr and post your review as a comment.

@enyst
enyst requested a review from all-hands-bot September 10, 2026 03:12
@openhands-ai

openhands-ai Bot commented Sep 10, 2026

Copy link
Copy Markdown

I'm on it! enyst can track my progress at all-hands.dev

enyst commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

This review comment was generated by an AI agent (OpenHands) on behalf of @enyst.

🔴 Needs improvement

[CRITICAL ISSUES]

  • [skills/github-pr-reviewer/scripts/main.py:650-655, skills/slack-channel-monitor/scripts/main.py:528-533] A renamed or deleted selected profile disables both automations instead of falling back. AUTOMATION_MODEL is a persisted profile name, so it can become stale. Both implementations call _fetch_llm_profile() without handling its 404; the exception then aborts conversation creation. This contradicts the repository's documented custom-automation contract in skills/openhands-automation/references/custom-automation.md:279-295, which requires falling back to the user's default LLM when the selected profile no longer exists. I reproduced this in both modules: a profile endpoint 404 propagates as HTTPError rather than selecting the default. Catch only the not-found case, use the already-fetched default agent_settings.llm, record the actual fallback profile/model, and keep authentication/server/malformed-response failures fatal.

  • [skills/github-pr-reviewer/scripts/main.py:458-477, skills/github-pr-reviewer/scripts/main.py:1057-1058] The successful GitHub path does not guarantee the feature it claims to provide. The script asks the LLM to append the footer, but _matching_review_exists() accepts any review by the bot at the head commit without inspecting its body. I reproduced a matching review whose body was review without provenance; _matching_review_exists() returned True, so the automation closes the record and emits no deterministic fallback. Prompt compliance is not an invariant. Make publication script-owned so the footer is appended before the GitHub request, or at minimum validate the matched review body and deterministically publish the missing provenance before marking the task closed.

[TESTING GAPS]

  • [skills/github-pr-reviewer/tests/test_main.py:533-601, tests/test_slack_channel_monitor.py:57-106] The new tests cover successful profile resolution, prompt text, and fallback comments, but not either failure above. Add regression tests for a selected-profile 404 in both modules and for a matching GitHub review that omits or alters the required footer.

All eight current PR checks pass. Local py_compile and git diff --check also pass; the focused reproductions above exercise the uncovered behavior directly.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM

This changes LLM selection and external output in two automation paths. There are no dependency changes or new external services, but stale profile state can stop both automations and the primary GitHub success path remains nondeterministic.

VERDICT:
Needs rework: The selected-profile fallback and successful-review provenance guarantee must be fixed before merge.

KEY INSIGHT:
Model provenance is audit metadata, so the deterministic automation layer - not the LLM prompt - must own and verify it.


Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

  1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
  2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
  3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

@openhands-ai

openhands-ai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Reviewed PR #547 and posted the review comment:

#547 (comment)

The review requests changes for two issues:

  • Deleted or renamed AUTOMATION_MODEL profiles return 404 and stop both automations instead of falling back to the default LLM.
  • The GitHub success path accepts any matching review without verifying the required provenance footer, so prompt noncompliance can silently omit it.

All 8 current PR checks are green. py_compile and git diff --check passed. Focused reproductions confirmed both findings. Local pytest and sync checks could not run because the existing environment lacks pytest and PyYAML.

@enyst

enyst commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Astra feedback: request changes, linked provider profiles lose resolved credentials.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show LLM provenance in PR-review and Slack automation output

3 participants