fix(agent): reset session state between reviews

PLAN.md

@@ -0,0 +1,42 @@
+## Solution plan
+
+**Issue:** [Agent session state is not cleared between reviews for the same user](https://github.com/jamjamgobambam/pathreview/issues/43)
+
+### Understand
+
+The agent orchestrator persists tool results into session state using the profile/session identifier, then merges new results into the previous state. The expected behavior is that a new review starts with clean tool state, while memoization can still be reused inside one review run. The actual behavior is that results from an older review can remain in the persisted session after a later review for the same profile no longer runs those tools.
+
+### Map
+
+Files I expect to touch:
+
+- `agent/orchestrator.py`
+- `agent/memory/session_store.py`
+- `tests/unit/test_orchestrator_session_state.py`
+- `docs/JOURNAL.md`
+
+Related files to inspect before changing production code:
+
+- `agent/memory/context_manager.py`
+- `core/services/review_service.py`
+- `api/routes/reviews.py`
+
+### Plan
+
+1. Add a focused failing unit test that reproduces two reviews for the same profile and proves stale tool results are not carried across the review boundary.
+2. Decide the smallest review-boundary API change: either clear the stored session at the start of a new review or scope session keys by a review-specific identifier.
+3. Update the orchestrator/session-store path so persisted state cannot merge old review results into a new review.
+4. Keep in-memory `ContextManager` memoization available only within a single orchestrator run or explicitly scoped review session.
+5. Run the relevant unit tests and update `docs/JOURNAL.md` with the final result in Week 9.
+
+### Inputs & outputs
+
+Input to the fix is a new review request for an existing profile/user, plus the profile data used to build the agent tool plan. The output should be freshly computed tool results for that review, with persisted session state containing only results that belong to the current review boundary.
+
+### Risks & unknowns
+
+The main risk is clearing too much state and removing useful within-review memoization. Another unknown is that `core/services/review_service.py` currently has placeholder orchestration logic, so I need to confirm where the real orchestrator will be wired before choosing the final public method signature.
+
+### Edge cases
+
+The fix should handle a second review that has fewer inputs than the first review, an empty tool plan, failed tool executions, repeated reviews for the same profile, and concurrent reviews that should not overwrite each other's session state.

agent/orchestrator.py

@@ -1,21 +1,26 @@
 """Plan-execute orchestrator for agent tools."""
 
 import time
+from typing import Any
+
 import structlog
-from typing import Optional
 
-from .memory.session_store import SessionStore
-from .memory.context_manager import ContextManager
 from .error_handling import retry_with_backoff
+from .memory.context_manager import ContextManager
+from .memory.session_store import SessionStore
 
 logger = structlog.get_logger()
 
 
 class Orchestrator:
     """Orchestrate tool execution with planning and memoization."""
 
-    def __init__(self, tools: dict, session_store: Optional[SessionStore] = None,
-                 tool_timeout: float = 30.0):
+    def __init__(
+        self,
+        tools: dict,
+        session_store: SessionStore | None = None,
+        tool_timeout: float = 30.0,
+    ):
         """Initialize orchestrator.
 
         Args:
@@ -40,39 +45,35 @@ def run(self, profile_id: str, profile_data: dict) -> dict:
         """
         logger.info("orchestrator_start", profile_id=profile_id)
 
+        # Keep tool memoization inside this review only.
+        self.context_manager = ContextManager()
+
         # Build execution plan
         plan = self._build_plan(profile_data)
 
-        # Load previous session state if available
-        session_state = {}
-        if self.session_store:
-            session_state = self.session_store.get(profile_id) or {}
-
         # Execute plan
         results = {}
         for tool_name, tool_input in plan:
             try:
                 result = self._execute_tool(tool_name, tool_input)
-                results[tool_name] = result.data if hasattr(result, 'data') else result
+                results[tool_name] = result.data if hasattr(result, "data") else result
 
                 logger.info("tool_executed", tool=tool_name, success=True)
 
             except Exception as e:
                 logger.error("tool_execution_failed", tool=tool_name, error=str(e))
                 results[tool_name] = {"error": str(e), "success": False}
 
-        # Persist state
+        # Persist only this review's state so stale tool results do not carry over.
         if self.session_store:
-            session_state.update(results)
-            self.session_store.set(profile_id, session_state)
+            self.session_store.set(profile_id, results)
 
-        logger.info("orchestrator_complete", profile_id=profile_id,
-                   tools_executed=len(results))
+        logger.info("orchestrator_complete", profile_id=profile_id, tools_executed=len(results))
 
         return {
             "profile_id": profile_id,
             "tool_results": results,
-            "cached_results": self.context_manager.get_all_results()
+            "cached_results": self.context_manager.get_all_results(),
         }
 
     def _build_plan(self, profile_data: dict) -> list[tuple[str, dict]]:
@@ -90,50 +91,47 @@ def _build_plan(self, profile_data: dict) -> list[tuple[str, dict]]:
         if profile_data.get("github_username"):
             for project in profile_data.get("projects", []):
                 if project.get("github_repo"):
-                    plan.append((
-                        "github_tool",
-                        {
-                            "github_username": profile_data["github_username"],
-                            "repo_name": project["github_repo"]
-                        }
-                    ))
+                    plan.append(
+                        (
+                            "github_tool",
+                            {
+                                "github_username": profile_data["github_username"],
+                                "repo_name": project["github_repo"],
+                            },
+                        )
+                    )
                     break  # Only process first repo for now
 
         # Tech detector (if files available)
         if profile_data.get("files"):
-            plan.append((
-                "tech_detector",
-                {"files": profile_data["files"]}
-            ))
+            plan.append(("tech_detector", {"files": profile_data["files"]}))
 
         # README scorer
         if profile_data.get("readme_content"):
-            plan.append((
-                "readme_scorer",
-                {"readme_content": profile_data["readme_content"]}
-            ))
+            plan.append(("readme_scorer", {"readme_content": profile_data["readme_content"]}))
 
         # Skill extractor
         if profile_data.get("resume_text"):
-            plan.append((
-                "skill_extractor",
-                {
-                    "resume_text": profile_data["resume_text"],
-                    "repo_metadata": profile_data.get("repo_metadata", {})
-                }
-            ))
+            plan.append(
+                (
+                    "skill_extractor",
+                    {
+                        "resume_text": profile_data["resume_text"],
+                        "repo_metadata": profile_data.get("repo_metadata", {}),
+                    },
+                )
+            )
 
         # Market analyzer (if skills detected)
         if plan:  # Only if other tools executed
-            plan.append((
-                "market_analyzer",
-                {"detected_skills": {}}  # Will be populated by context
-            ))
+            plan.append(
+                ("market_analyzer", {"detected_skills": {}})  # Will be populated by context
+            )
 
         logger.info("plan_built", plan_size=len(plan))
         return plan
 
-    def _execute_tool(self, tool_name: str, tool_input: dict):
+    def _execute_tool(self, tool_name: str, tool_input: dict) -> Any:
         """Execute a single tool with retry and memoization.
 
         Args:
@@ -172,7 +170,12 @@ def _execute_tool(self, tool_name: str, tool_input: dict):
             logger.error("tool_execution_error", tool=tool_name, error=str(e))
             raise
 
-    def _execute_with_timeout(self, tool, tool_input: dict, timeout: Optional[float] = None):
+    def _execute_with_timeout(
+        self,
+        tool: Any,
+        tool_input: dict,
+        timeout: float | None = None,
+    ) -> Any:
         """Execute tool with timeout.
 
         Args:

docs/JOURNAL.md

@@ -0,0 +1,124 @@
+## Week 7 — Issue selection
+
+**Issue link:** https://github.com/jamjamgobambam/pathreview/issues/43
+
+**Issue title:** Agent session state is not cleared between reviews for the same user
+
+**Tier:** [X] Tier 1  [ ] Tier 2  [ ] Tier 3
+
+**Selection notes:** I chose Tier 1 because this is my first open-source contribution. The issue is focused on the session store, the expected behavior is clear, and the scope feels manageable within the estimated time.
+
+**Problem summary:**
+`session_store.py` currently caches agent state using only the user ID, allowing tool results to outlive the review that produced them. When the same user updates their portfolio and requests another review, the orchestrator reuses stale cached results instead of running the tools against the updated content. This can produce feedback based on the user's previous portfolio state and makes repeat reviews unreliable. The fix should clear or scope cached state at review boundaries so each new review recomputes its tool results while state remains reusable within a single review.
+
+**Branch name:** `fix/43-uncleared-session-state`
+
+**Setup confirmation:** [X] App runs locally at localhost:5173
+
+**Cohort ledger:** [X] Issue added to cohort ledger
+
+**Reproduction note:**
+I reproduced the stale session-state issue locally with a fake in-memory session store and fake agent tools. I ran two reviews with the same `profile_id`: the first review included `readme_content`, so `readme_scorer` and `market_analyzer` were saved into session state; the second review had no tool inputs, so no tools ran, but the persisted session still kept the old `readme_scorer` and `market_analyzer` entries. This confirms the review boundary does not clear or scope stored tool results.
+
+Reproduction command:
+
+```powershell
+$env:PYTHONPATH='C:\Users\hissa\Desktop\pathreview\pathreview'; C:\Users\hissa\Desktop\pathreview\pathreview\.venv\Scripts\python.exe .\work\reproduce_pathreview_session_state.py
+```
+
+Observed output:
+
+```text
+first tool_results: ['market_analyzer', 'readme_scorer']
+second tool_results: []
+persisted session keys after second review: ['market_analyzer', 'readme_scorer']
+stale readme result remains: True
+```
+
+## Week 8 — Reproduction & solution planning
+
+**Reproduction commit link:** [ef28ca4](https://github.com/HissanT/pathreview/commit/ef28ca45084dc0c1b8f36bee477bdda01f73db92)
+
+**Reproduction summary:**
+I reproduced the issue locally by running two orchestrator reviews with the same `profile_id` and a fake in-memory session store. The first review saved `readme_scorer` and `market_analyzer` results, and the second review had no tool inputs, but the persisted session still kept those old tool results.
+
+**PLAN.md link:** [PLAN.md](https://github.com/HissanT/pathreview/blob/fix/43-uncleared-session-state/PLAN.md)
+
+**Walkthrough video (recommended):** Not recorded; the reproduction steps and observed output are documented in the reproduction commit.
+
+**Blockers or open questions:**
+The main open question is whether Week 9 should clear session state at the start of each review or add a review-scoped session key once the real orchestration path is wired into `process_review`.
+
+## Week 9 — Solution building & PR submission
+
+### Check-in 1 (mid-week)
+
+**Current progress:**
+I added a focused failing unit test for issue #43 in `tests/unit/test_orchestrator_session_state.py`. This completed the first PLAN.md sub-task: proving that old tool results could stay in the saved session when the same profile was reviewed again.
+
+**Next steps:**
+Next I planned to update `agent/orchestrator.py` so each review starts with fresh tool state and only saves the current review's results. After that, I planned to rerun the focused tests and the project checks.
+
+**Blockers:**
+The full unit suite and full check commands had pre-existing failures before I changed the code.
+
+---
+
+### Check-in 2 (end of week)
+
+**PR link:** [ascherj/pathreview#855](https://github.com/ascherj/pathreview/pull/855)
+
+**Branch:** `fix/43-uncleared-session-state`
+
+**What you built:**
+I fixed the stale session-state bug in `agent/orchestrator.py`. Each orchestrator run now starts with a fresh in-memory context cache, and the session store saves only the current review's tool results instead of merging them into old saved results.
+
+**Tests added or updated:**
+I added `tests/unit/test_orchestrator_session_state.py`. The tests cover two cases: a second review for the same profile does not keep old tool results, and the same tool input is recomputed for a new review instead of coming from the previous review's memoized cache.
+
+**Self-review confirmation:** [X] make check passes with documented pre-existing failures  [X] make test-unit passes with documented pre-existing failures
+
+**Draft PR feedback received from:** none
+
+**Validation notes:**
+The focused orchestrator test passed with 2 tests, targeted ruff passed on the touched files, and targeted black passed on the touched files. Before the fix, the full unit suite had 53 failures; after the fix, it still had 53 failures, and the 2 new orchestrator tests pass.
+
+## Week 10 — Iteration & reflection
+
+### Reviewer feedback
+
+**Feedback received:** [ ] Yes  [X] No — still awaiting review
+
+**Summary of feedback:**
+No reviewer feedback came in for Summer 2026, so there were no requested changes to address.
+
+**How you responded:**
+
+---
+
+### Reflection
+
+**What was harder than you expected?**
+The hardest part was understanding where the session state actually lived and how it moved through the app. At first the issue sounded like a simple cache problem, but it was not obvious which cache was causing it. I had to trace the flow through `agent/orchestrator.py`, `agent/memory/context_manager.py`, and `agent/memory/session_store.py` before the bug made sense.
+
+What surprised me was that the problem was not that the tools were broken. The tools could run correctly, but the app was saving their results in a way that let old results stay around. The orchestrator loaded previous session data, added the new tool results on top of it, and saved it again. That meant a second review for the same profile could still have information from the first review. Figuring that out took more careful reading than I expected.
+
+**What did you learn about working in a large codebase?**
+I learned that small bugs can come from the way files connect, not just from one bad line of code. In my own projects I usually know the whole flow already because I wrote it. In someone else's codebase, I had to slow down and understand the existing structure before changing anything.
+
+This project also showed me that a fix should be as small as possible. It would have been easy to rewrite more of the session store or change the whole review flow, but that would have added risk. The better approach was to find the exact boundary between one review and the next review, then make sure old state did not cross that boundary. I also learned that tests are important for explaining the fix. The new test makes the bug clear without needing a long explanation.
+
+**How did AI tools help — and where did they fall short?**
+AI tools helped me move faster when searching the codebase and understanding unfamiliar files. They were useful for finding session-related code, explaining what the orchestrator was doing, and helping me turn the issue into a clear reproduction and unit test. They also helped me explain the bug in plain language, which made the journal and PR easier to write.
+
+Where AI fell short was project-specific judgment. AI could suggest possible fixes, but it could not know which fix was safest without actually reading this repo and checking the behavior. I still had to reproduce the issue locally, compare the before-and-after test results, and make sure I was not changing unrelated parts of the app. The AI was a useful guide, but I still had to verify everything myself.
+
+**What would you do differently if you started over?**
+If I started over, I would write the failing unit test earlier. Once I had the test, the issue became much easier to understand because it showed the problem directly: one review saved tool results, and the next review could still keep those old results. That test also made the fix easier to trust.
+
+I would also map the code path earlier before writing too much planning text. I understood the general issue from the beginning, but I could have found the exact session read and write points sooner. Starting with the exact files and exact behavior would have made the process smoother.
+
+**What are you most proud of from this module?**
+I am most proud that I reproduced the bug clearly before fixing it. I did not just change code based on the issue description. I proved the problem locally with fake tools and a fake session store, then turned that reproduction into a real unit test.
+
+I am also proud that the final fix is simple. The main idea is easy to explain: a new review should start fresh, and it should only save the current review's tool results. That makes the app more reliable without changing unrelated parts of the system.