Skip to content

test: sequence the advisor mock with ordered handlers - #64

Merged
elkaix merged 1 commit into
mainfrom
fix/advisor-test-conditionals
Aug 12, 2026
Merged

test: sequence the advisor mock with ordered handlers#64
elkaix merged 1 commit into
mainfrom
fix/advisor-test-conditionals

Conversation

@elkaix

@elkaixelkaix commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

packages/agent-core/test/session/session-advisor.test.ts emitted two vitest(no-conditional-in-test) warnings from a call-counter mock that branched on which rawGenerate call it was handling.

A reviewer raised this on #59 and I declined it incorrectly, citing an oxlint --quiet run as evidence the rule did not fire. That flag suppresses warnings, so the evidence was an artifact of the command rather than a fact about the code. This is the follow-up.

Approach

The three-call sequence is now expressed with ordered mockImplementationOnce handlers plus a trailing default, so an unexpected fourth call falls through to the real implementation instead of returning undefined.

No production code and no assertions changed.

Test plan

  • pnpm --filter @pythoughts/agent-core exec vitest run test/session/session-advisor.test.ts — all pass.
  • npx oxlint packages/agent-core/test/session/session-advisor.test.ts (no --quiet) — 2 warnings before, 0 after.
  • Red proof that the test still catches its bug: restoring the mid-turn #deliverPending() call in session-advisor.ts fails the test with expected 1 to be +0 on callsWhileActive.

[skip changeset] — tests-only change under packages/agent-core/test/. No production source is touched and nothing enters the CLI bundle, so per the repo's changeset rules there is no user-visible change to record.

Summary by CodeRabbit

  • Tests
    • Updated mid-turn advisor coverage to validate review and active-turn behavior using explicit response sequencing.
    • Improved test reliability by removing dependence on call-count state.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a949615-afb6-423f-88b8-82ad7dd48b6e

📥 Commits

Reviewing files that changed from the base of the PR and between 7fc36fd and 855bd08.

📒 Files selected for processing (1)
  • packages/agent-core/test/session/session-advisor.test.ts

📝 Walkthrough

Walkthrough

The mid-turn advisor test replaces call-counter state with explicit rawGenerate implementations. The implementations preserve review and active-turn gating, then fall back to the original generator.

Changes

Mid-turn advisor test

Layer / File(s)Summary
Explicit rawGenerate sequencing
packages/agent-core/test/session/session-advisor.test.ts
The mock defines separate behaviors for the first call, review-gated call, active-turn-gated call, and later calls.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title uses the test: prefix, starts with an imperative verb, describes the test mock change, and is within 72 characters.
Description check✅ PassedThe description explains the problem, approach, tests, issue context, and changeset decision, but it does not use every template heading or checklist item.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@pythoughts/pythinker-code@855bd08
npx https://pkg.pr.new/@pythoughts/pythinker-code@855bd08

commit: 855bd08

@elkaix
elkaix merged commit ca2302a into mainAug 12, 2026
13 checks passed
@elkaix
elkaix deleted the fix/advisor-test-conditionals branch August 12, 2026 08:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@elkaix