FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid duplicate model flags for session runs by grp06 · Pull Request #357 · self-evolving/repo · GitHub

/ repo Public template

Avoid duplicate model flags for session runs - #357

Merged
lolipopshock merged 2 commits into
self-evolving:mainfrom
grp06:fix-session-model-flag
May 26, 2026
Merged

Avoid duplicate model flags for session runs#357
lolipopshock merged 2 commits into
self-evolving:mainfrom
grp06:fix-session-model-flag

Conversation

grp06 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary:

  • Use session setup as the single model-selection path for named ACPX sessions.
  • Keep global --model on no-session exec commands.
  • Add coverage for persistent and transient session command construction.

Testing:

  • from the package directory: npm ci
  • from the package directory: npm run build
  • from the package directory: node --test dist/__tests__/acpx-adapter.test.js
  • from the package directory: npm test
  • git diff --check

Closes #342

sepo-agent-app Bot commented May 26, 2026
edited
Loading

Copy link
Copy Markdown

Sepo is dispatching follow-up automation.

Source Next Target Round Status
orchestrate review PR #357 2 / 40 Dispatched

Reason: agent planner selected review: PR #357 is open, labeled for agent review, and there is no latest review synthesis or concrete fix-pr finding yet; run review first.

Copy link
Copy Markdown

Rubrics Review

Total Score Verdict Rubrics Scored
96 PASS 5
Dimension Rubric Result Score Evidence
coding_workflow / generic Prefer explicit structured inputs pass 19/20 The change keeps model selection on existing typed inputs (model, sessionName, isExecRoute) and removes the duplicate global model path for named sessions via usesNamedSession in acpx-adapter.ts.
coding_workflow / generic Reuse existing code pass 19/20 The implementation changes the central buildAcpxArgs helper and continues to rely on existing buildSessionSetupCommands for session model setup in acpx-adapter.ts.
coding_workflow / generic Make surgical changes pass 20/20 The PR touches only .agent/src/acpx-adapter.ts and .agent/src/__tests__/acpx-adapter.test.ts, with a minimal conditional change plus focused assertions.
coding_style / generic Prefer simple implementations pass 20/20 The behavior is implemented with a straightforward usesNamedSession boolean rather than adding configuration, new routing, or alternate model-selection machinery.
coding_workflow / generic Separate diagnostics from continuity pass 18/20 Transient session coverage still verifies track-only exec runs avoid stable session reuse while now applying set model during transient session setup and omitting global --model from the prompt command in acpx-adapter.test.ts.

Notes

  • The PR directly resolves the issue’s intended contract: named ACPX sessions use session setup for model selection, while no-session exec commands still receive global --model.
  • Coverage is focused and relevant: persistent named-session argument construction is checked in buildAcpxArgs, no-session exec model behavior remains covered in buildAcpxArgs, and transient session command construction is covered through runAcpx.
  • No rubric-specific follow-up needed.

Findings

  • INFO: The implementation satisfies the applicable selected rubrics. No blocking or warning-level rubric gaps found.

Final Rubric Verdict

PASS

Copy link
Copy Markdown

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

Review

The independent Claude, Codex, and rubrics reviews all found no BLOCKING or WARNING issues. Current PR head 6fdfcc65802ed81b81a60f39b901675619439369 matches that conclusion: the diff suppresses duplicate model flags for named-session prompt paths while preserving exec behavior.

Issue Severity Description
No actionable issues INFO The implementation is minimal, aligned with issue #342, and covered by focused tests for named-session argument construction and transient session setup.

Progress

  • Linked issue Clarify ACPX model selection for named/transient sessions #342 was reviewed and the PR implements option 1 from that issue: use global --model only for no-session exec runs.
  • Existing PR conversation has a rubrics PASS and no blocking discussion.
  • Existing inline comments and review threads were re-fetched; none were present.
  • No inline comments were posted, replied to, minimized, or resolved.

Recommended Next Step

NO_AUTOMATED_ACTION: No unresolved actionable branch-change work remains for an automated fix-pr pass.

Final Verdict

SHIP

Action Items

  • No required branch changes.

lolipopshock merged commit f9fab3c into self-evolving:main May 26, 2026
11 of 13 checks passed

Copy link
Copy Markdown

Rubrics Update

No changes were committed to agent/rubrics from PR #357.

no rubric changes

I found no trusted contributor feedback in PR #357 that warranted a durable rubric update. The PR conversation contained only bot comments/reviews, no inline review comments, and the linked issue #342 was bot-authored. The PR author was CONTRIBUTOR; the merge actor/requester lolipopshock did not author an explicit rubric preference in the conversation.

Existing rubrics already cover the implementation themes reflected by the PR: explicit structured inputs, surgical changes, simple implementation, and separating diagnostics from continuity. No files were changed under agent-rubrics.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent/review agent Handled by the agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify ACPX model selection for named/transient sessions

2 participants


Back | FazBrowse Home | New Git URL