refactor(core): unify ReactAgent context loading via TaskContextPackager

[frankbria]

Summary

Implements #431: [Phase 4] Unify ReactAgent context assembly with TaskContextPackager

• Added load_context(task_id) -> TaskContext method to TaskContextPackager for internal agent use
• Updated ReactAgent.run() to use TaskContextPackager instead of directly instantiating ContextLoader
• Removed ContextLoader import from react_agent.py — all context loading now goes through TaskContextPackager
• Updated ~60 test mocks across 4 test files to reflect the new code path

Acceptance Criteria

• ReactAgent uses TaskContextPackager internally for context loading
• ReactAgent's 3-layer prompt format is preserved (no behavior change)
• ContextLoader import can be removed from react_agent.py
• Integration test confirms same effective prompt content

Test Plan

• 3 new tests for load_context() (returns TaskContext, delegates to loader, matches build())
• All 178 react agent tests pass with updated mocks
• Full v2 test suite: 2142 passed, 0 failed
• Linting clean (ruff)

Implementation Notes

• Named the method load_context() instead of build_agent_context() (as in the traycer plan) because build_agent_context() already exists from [Phase 4] Task Context Packager for External Agents #410 and returns AgentContext for external adapters
• Zero behavior change — only the ownership of context loading moves from ReactAgent to TaskContextPackager

Closes #431

Summary by CodeRabbit

Release Notes

• Refactor

  • Restructured internal context loading mechanism to improve code organization and maintainability of the agent framework.

• Tests

  • Added comprehensive test coverage for the new context loading approach and updated existing tests to validate the refactored architecture.

[coderabbitai]

Walkthrough

Introduced new public method load_context() to TaskContextPackager that exposes internal context loading. Refactored ReactAgent to use TaskContextPackager instead of ContextLoader directly, removing unnecessary import. Updated all related test suites to mock and call the new method accordingly.

Changes

Cohort / File(s)	Summary
Context Loading API codeframe/core/context_packager.py	Added public load_context(task_id: str) -> TaskContext method to TaskContextPackager, exposing internal ContextLoader.load() as a public interface for internal agent use.
ReactAgent Integration codeframe/core/react_agent.py	Replaced ContextLoader import and direct loader.load(task_id) call with TaskContextPackager and packager.load_context(task_id). No change to surrounding logic or control flow.
Test Suites tests/core/test_context_packager.py, tests/core/test_react_agent.py, tests/core/test_react_agent_compaction.py, tests/core/test_react_agent_escalation.py	Updated mock targets from ContextLoader to TaskContextPackager. Replaced mocked method calls from load() to load_context(). Added new TestLoadContext test suite validating the new public method. All test assertions remain consistent with updated mocking strategy.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A cleaner path through context's maze,
TaskContextPackager lights the way,
No ContextLoader in ReactAgent's gaze—
Unified loading saves the day! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: refactoring ReactAgent to use TaskContextPackager for unified context loading instead of direct ContextLoader usage.
Linked Issues check	✅ Passed	All acceptance criteria from issue #431 are met: ReactAgent uses TaskContextPackager, 3-layer prompt format preserved, ContextLoader import removed, and integration tests pass.
Out of Scope Changes check	✅ Passed	All changes directly support the refactoring objective. New method load_context() enables the unified path, ReactAgent imports/calls TaskContextPackager, and test updates align with the implementation.
Docstring Coverage	✅ Passed	Docstring coverage is 94.32% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/issue-431-unify-react-context

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[claude]

PR #435 Review: Unify ReactAgent context loading via TaskContextPackager

Overall: Clean, well-scoped refactor. Approve with minor observations.

What is Working Well

• Zero behavior change - load_context() is a one-line delegation to self._loader.load(), so there is no execution path risk.
• Docstring clarity - The three-way distinction between build(), build_agent_context(), and load_context() is clearly documented. Good decision to name it load_context to avoid collision with build_agent_context() from issue [Phase 4] Task Context Packager for External Agents #410.
• Test coverage is thorough - 3 new targeted tests in TestLoadContext, all 178 react agent tests updated and passing, full v2 suite green.

Minor Observations

1. test_returns_same_context_as_build test name is slightly misleading

The test asserts raw_ctx is packaged.context, which passes because both calls share the same mock loader instance. In production, calling load_context() and build() back-to-back would invoke _loader.load() twice. The test is not wrong, but the name implies semantic equivalence when it is really just testing mock identity. Consider a name like test_load_context_uses_same_loader_as_build to better reflect what is verified.

2. Fresh packager instantiation on every run() call

The packager is created fresh on each run() invocation - same pattern as ContextLoader was before, so no regression. Worth noting for future work: if TaskContextPackager becomes heavier to initialize, moving it to ReactAgent.__init__ would be straightforward. Low priority.

3. load_context accesses self._loader directly

Fine within the class and consistent with existing conventions in the file.

Summary

This PR accomplishes its stated goal: ContextLoader is no longer a direct dependency of react_agent.py, and all context loading now routes through TaskContextPackager. The test updates across 4 files are mechanical and correct. The minor observation on the test name is the only thing worth considering - everything else looks good.

Ready to merge.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/core/test_react_agent.py (1)

498-507: ⚠️ Potential issue | 🟠 Major

Exception mocks still target the old method name.

At Line 506, Line 886, and Line 1628, the tests set mock_ctx_loader.return_value.load.side_effect, but ReactAgent.run() now calls load_context(). This means the intended exception path is not being exercised reliably.

🔧 Suggested fix

-        mock_ctx_loader.return_value.load.side_effect = RuntimeError("DB corrupt")
+        mock_ctx_loader.return_value.load_context.side_effect = RuntimeError("DB corrupt")

-        mock_ctx_loader.return_value.load.side_effect = RuntimeError("boom")
+        mock_ctx_loader.return_value.load_context.side_effect = RuntimeError("boom")

-        mock_ctx_loader.return_value.load.side_effect = RuntimeError("boom")
+        mock_ctx_loader.return_value.load_context.side_effect = RuntimeError("boom")

Also applies to: 877-887, 1620-1629

🤖 Prompt for AI Agents

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

In `@tests/core/test_react_agent.py` around lines 498 - 507, The tests mock
TaskContextPackager but set side effects on the old method name 'load'; update
all occurrences where tests set mock_ctx_loader.return_value.load.side_effect to
instead set mock_ctx_loader.return_value.load_context.side_effect so the
RuntimeError (e.g., "DB corrupt") is raised when ReactAgent.run() invokes
load_context(); update the same change at the other test sites that target
TaskContextPackager mocks to ensure the exception path is actually exercised for
ReactAgent.run().

🤖 Prompt for all review comments with AI agents

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

Outside diff comments:
In `@tests/core/test_react_agent.py`:
- Around line 498-507: The tests mock TaskContextPackager but set side effects
on the old method name 'load'; update all occurrences where tests set
mock_ctx_loader.return_value.load.side_effect to instead set
mock_ctx_loader.return_value.load_context.side_effect so the RuntimeError (e.g.,
"DB corrupt") is raised when ReactAgent.run() invokes load_context(); update the
same change at the other test sites that target TaskContextPackager mocks to
ensure the exception path is actually exercised for ReactAgent.run().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b1683767-7e1a-4b7d-bccd-88739b5da02b

📥 Commits

Reviewing files that changed from the base of the PR and between 3bcf94b and f6675b4.

📒 Files selected for processing (6)

• codeframe/core/context_packager.py
• codeframe/core/react_agent.py
• tests/core/test_context_packager.py
• tests/core/test_react_agent.py
• tests/core/test_react_agent_compaction.py
• tests/core/test_react_agent_escalation.py