test: session — loaded() and filterCompacted() coverage

[anandgupta42]

Summary

What does this PR do?

1. InstructionPrompt.loaded() — src/session/instruction.ts (7 new tests)

This pure function extracts loaded file paths from session message history, used by resolve() to prevent duplicate AGENTS.md/CLAUDE.md injection. Zero tests existed. If broken, users get duplicate instructions in their context window — wasting tokens and potentially confusing the model with repeated directives. New coverage includes:

• Empty set returned for messages with no tool parts
• Correct extraction from completed read tool parts with metadata.loaded
• Compacted parts skipped (state.time.compacted guard)
• Non-read tools ignored even when they have loaded metadata (e.g., bash tool)
• Non-completed read tools ignored (running, error states)
• Non-string entries in the loaded array filtered out (defends against z.any() metadata)
• Deduplication across multiple messages via Set semantics

2. MessageV2.filterCompacted() — src/session/message-v2.ts (5 new tests)

This async function processes a message stream to find the compaction boundary — the point where old messages were summarized and can be trimmed. Zero tests existed. If broken, compacted sessions either include stale pre-compaction content (context overflow) or lose valid messages. New coverage includes:

• All messages returned (reversed) when no compaction point exists
• Correct stop at compaction boundary with proper slice reversal
• Compaction part alone insufficient — requires matching completed assistant response
• No false stop when assistant has no summary/finish fields
• Errored assistant messages (msg.info.error) do NOT mark parent as completed — isolates the !error guard with summary: true, finish: "stop" present

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage for session module

How did you verify your code works?

bun test test/session/instruction.test.ts   # 13 pass (6 existing + 7 new)
bun test test/session/message-v2.test.ts    # 23 pass (18 existing + 5 new)

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_01TGBJo6pqwxz6qh72s79wCg

Summary by CodeRabbit

• Tests
  • Added comprehensive test coverage for instruction prompt loading behavior and message filtering with compaction boundary detection.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Two test suites were added to enhance coverage for core functionality. The first tests InstructionPrompt.loaded() behavior across nine scenarios, including handling of completed/non-completed tool parts, file path deduplication, and metadata filtering. The second tests MessageV2.filterCompacted() with five scenarios covering message filtering and stopping behavior based on compaction boundaries and assistant completion states.

Changes

Cohort / File(s)	Summary
InstructionPrompt.loaded test suite packages/opencode/test/session/instruction.test.ts	Added describe("InstructionPrompt.loaded") with helper factories and nine test cases covering scenarios: empty tool parts, extraction of file paths from completed read tool parts only, filtering based on state.metadata.loaded presence, exclusion of compacted items, type-checking, and deduplication across messages.
MessageV2.filterCompacted test suite packages/opencode/test/session/message-v2.test.ts	Added describe("session.message-v2.filterCompacted") with helper builders for user/assistant messages and five test cases validating filtering behavior: no-boundary scenarios, stopping at compaction boundaries with completed sets, assistant error handling preventing stops, and comprehensive boundary+completion checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hops with glee through test arrays,
Helper factories brighten my days,
Nine scenarios, then five more to explore,
Coverage expanding from floor to door!
Loaded paths and filters so clean,
Best tested code the codebase has seen! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and concisely summarizes the main change: adding test coverage for two session functions (loaded() and filterCompacted()).
Description check	✅ Passed	The description comprehensively covers all required sections: Summary (what changed and why), Test Plan (verification with test commands), and Checklist (tests added, no documentation/CHANGELOG updates needed).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/test-session-loaded-filtercompacted-01TGBJo6pqwxz6qh72s79wCg

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

packages/opencode/test/session/message-v2.test.ts (1)

1004-1007: Strengthen ordering assertions to full sequence checks.

Line [1004] and Line [1020] currently validate length plus endpoints only; a middle-order regression could still pass.

Suggested assertion tightening

-    expect(result.length).toBe(5)
-    expect(result[0].info.id).toBe("u1")
-    expect(result[4].info.id).toBe("u3")
+    expect(result.map((m) => m.info.id)).toStrictEqual(["u1", "a1", "u2", "a2", "u3"])

-    expect(result.length).toBe(5)
-    expect(result[0].info.id).toBe("u1")
-    expect(result[4].info.id).toBe("u3")
+    expect(result.map((m) => m.info.id)).toStrictEqual(["u1", "a1", "u2", "a2", "u3"])

Also applies to: 1020-1023

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/session/message-v2.test.ts` around lines 1004 - 1007,
The test only asserts result.length and the first/last ids, which allows
middle-order regressions; update the assertions to verify the full ordering by
comparing the sequence of ids extracted from result (using result.map(r =>
r.info.id)) against the expected array of ids so both tests (the block around
result.length/asserts and the similar block at lines 1020-1023) assert the
entire ordered sequence rather than just endpoints.

packages/opencode/test/session/instruction.test.ts (1)

14-108: Optional: consolidate message/part test builders shared across session tests.

The helper constructors here overlap with fixture patterns in packages/opencode/test/session/message-v2.test.ts; extracting a small shared builder module would reduce drift when MessageV2 shapes evolve.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/session/instruction.test.ts` around lines 14 - 108,
Extract the overlapping test builders (makeUserMsg, readToolPart,
nonReadToolPart) into a small shared test helper module and import it from both
session tests; move the implementations out of
packages/opencode/test/session/instruction.test.ts into e.g. a new helper that
exports makeUserMsg, readToolPart, nonReadToolPart and accepts/sessionID or uses
a shared sid fixture so types (MessageV2.WithParts, MessageV2.ToolPart) and
utilities (MessageID.make, PartID.make) remain correct; update
instruction.test.ts and message-v2.test.ts to import these helpers instead of
duplicating logic and run tests to ensure no type regressions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/session/instruction.test.ts`:
- Around line 14-108: Extract the overlapping test builders (makeUserMsg,
readToolPart, nonReadToolPart) into a small shared test helper module and import
it from both session tests; move the implementations out of
packages/opencode/test/session/instruction.test.ts into e.g. a new helper that
exports makeUserMsg, readToolPart, nonReadToolPart and accepts/sessionID or uses
a shared sid fixture so types (MessageV2.WithParts, MessageV2.ToolPart) and
utilities (MessageID.make, PartID.make) remain correct; update
instruction.test.ts and message-v2.test.ts to import these helpers instead of
duplicating logic and run tests to ensure no type regressions.

In `@packages/opencode/test/session/message-v2.test.ts`:
- Around line 1004-1007: The test only asserts result.length and the first/last
ids, which allows middle-order regressions; update the assertions to verify the
full ordering by comparing the sequence of ids extracted from result (using
result.map(r => r.info.id)) against the expected array of ids so both tests (the
block around result.length/asserts and the similar block at lines 1020-1023)
assert the entire ordered sequence rather than just endpoints.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0b6586ad-8fdd-4cb0-9fc6-e2c6fc7e5090

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb13d4 and 7f9aad5.

📒 Files selected for processing (2)

• packages/opencode/test/session/instruction.test.ts
• packages/opencode/test/session/message-v2.test.ts

[anandgupta42]

Consolidated into #403