Skip to content

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
Copy link
Copy Markdown
Contributor

@grp06 grp06 commented May 26, 2026

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
Copy link
Copy Markdown

sepo-agent-app Bot commented May 26, 2026

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.

@sepo-agent-app
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

@sepo-agent-app
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 lolipopshock merged commit f9fab3c into self-evolving:main May 26, 2026
11 of 13 checks passed
@sepo-agent-app
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.

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