Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
|
This comment was posted by an AI agent (OpenHands). |
|
@OpenHands /codereview this pr and post your review as a comment. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
This review comment was generated by an AI agent (OpenHands) on behalf of @enyst. 🔴 Needs improvement [CRITICAL ISSUES]
[TESTING GAPS]
All eight current PR checks pass. Local [RISK ASSESSMENT]
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: KEY INSIGHT:
|
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Astra feedback: request changes, linked provider profiles lose resolved credentials. |
Address profile fallback and successful-review footer findings from #547. Fail clearly if linked credentials need the accompanying Agent Server fix. Co-authored-by: openhands <openhands@all-hands.dev>
The active profile pointer can drift from the concrete default LLM settings. Label 404 fallback as default instead of claiming the named profile was read. Co-authored-by: openhands <openhands@all-hands.dev>
|
📁 PR Artifacts Notice This PR contains a |
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
|
This comment was posted by an AI agent (OpenHands). |
Co-authored-by: openhands <openhands@all-hands.dev>
Resolve AUTOMATION_MODEL for both scripts and preserve the complete LLM configuration. Fall back only for an absent or missing selection; report the actual default model without assuming the active-profile pointer. Add HTTP regressions, regenerate bundle 1.0.3, and explain selection, credential handling, and the server dependency in .pr/selected-profile.html. Co-authored-by: openhands <openhands@all-hands.dev>
|
@OpenHands please understand this pr, merge main into it and fix the conflicts; push. then take a step back and /codereview . feel free to read previous reviews. post your feedback on pr. |
|
I'm on it! enyst can track my progress at all-hands.dev |
Preserve the profile-backed catalog worker and shared GitHub transport alongside the provenance-aware legacy reviewer. Regenerate catalogs at reviewer bundle 1.1.2 and adapt the regression test imports and prompt assertions. Co-authored-by: openhands <openhands@all-hands.dev>
|
🔴 Needs improvement This review was generated by an AI agent (OpenHands) on behalf of @enyst. Merged [CRITICAL ISSUES]
[TESTING GAPS]
Earlier feedback and rollout dependency The selected-profile 404 fallback and deterministic footer repair are now implemented and tested on the legacy path; those earlier findings should not simply be repeated unchanged. The provider-linked credential concern remains a rollout dependency, however: software-agent-sdk#4952 is still open, and current upstream Validation
[RISK ASSESSMENT]
VERDICT: ❌ Needs rework before merge. KEY INSIGHT: Provenance must be bound to the actual conversation and the exact artifact it published, not inferred from a shared prompt or the latest review by a bot.
|
|
Merged Posted the code review, identifying two blockers:
Validation: 55 local tests passed; CI: 995 passed, 24 skipped. The SDK-skill sync check remains red because generated documentation is stale relative to upstream. The review also notes the outstanding backend dependency for linked profiles. No fixes to the review findings were made. |
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.
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
LLM profile: <profile> · Model: <model>.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 --checkExpected result:
802 passed, 24 skipped. The sync check emits one pre-existing, non-blocking coverage warning forplugins/issue-duplicate-checker.Video/Screenshots
Not applicable; this changes generated GitHub/Slack text output and includes automated request-body assertions.
Notes
/api/settingsplaintext request is required to serialize the child Agent's LLM credential back to the same authenticated Agent Server; this PR no longer adds named-profile plaintext reads.enhancementand markedready-for-devafter confirming it already contained the required Desired Behavior and Acceptance Criteria sections.This pull request was created by an AI agent (OpenHands) on behalf of the user.
@enyst can click here to continue refining the PR