From c511b8bcb1df7ee15933f50ae6e380863d5998f9 Mon Sep 17 00:00:00 2001 From: ec6862 Date: Mon, 27 Jul 2026 16:49:25 -0400 Subject: [PATCH 1/8] chore: set up initial project structure --- JOURNAL.md | 0 frontend/package-lock.json | 10 ---------- 2 files changed, 10 deletions(-) create mode 100644 JOURNAL.md diff --git a/JOURNAL.md b/JOURNAL.md new file mode 100644 index 000000000..e69de29bb diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 7efac84d9..12b1f52e2 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -99,7 +99,6 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -449,7 +448,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" }, @@ -473,7 +471,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" } @@ -1561,7 +1558,6 @@ "integrity": "sha512-jp2P3tQMSxWugkCUKLRPVUpGaL5MVFwF8RDuSRztfwgN1wmqJeMSbKlnEtQqU8UrhTmzEmZdu2I6v2dpp7XIxw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.18.0" } @@ -1579,7 +1575,6 @@ "integrity": "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/prop-types": "*", "csstype": "^3.2.2" @@ -1972,7 +1967,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -3507,7 +3501,6 @@ "integrity": "sha512-L88oL7D/8ufIES+Zjz7v0aes+oBMh2Xnh3ygWvL0OaICOomKEPKuPnIfBJekiXr+BHbbMjrWn/xqrDQuxFTeyA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@asamuzakjp/dom-selector": "^2.0.1", "cssstyle": "^4.0.1", @@ -4094,7 +4087,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", "license": "MIT", - "peer": true, "dependencies": { "loose-envify": "^1.1.0" }, @@ -4107,7 +4099,6 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.3.1.tgz", "integrity": "sha512-5m4nQKp+rZRb09LNH59GM4BxTh9251/ylbKIbpe7TpGxfJ+9kv6BLkLBXIjjspbgbnIBNqlI23tRnTWT0snUIw==", "license": "MIT", - "peer": true, "dependencies": { "loose-envify": "^1.1.0", "scheduler": "^0.23.2" @@ -4771,7 +4762,6 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", From 51eab14a0e3e45b1f4484ea650d34b5a55e3e456 Mon Sep 17 00:00:00 2001 From: ec6862 Date: Mon, 27 Jul 2026 17:49:01 -0400 Subject: [PATCH 2/8] docs: add Week 7 deliverables to journal --- JOURNAL.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/JOURNAL.md b/JOURNAL.md index e69de29bb..73dc7801c 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -0,0 +1,56 @@ +## Week 7 — Issue selection + +**Issue link:** https://github.com/ascherj/pathreview/issues/43 + +**Issue title:** Agent session state is not cleared between reviews for the same user #43 +**Tier:** [x] Tier 1 [ ] Tier 2 [ ] Tier 3 + +**Problem summary:** +[In 3–5 sentences, in your own words: what the issue is (not a copy-paste of +the title), what is currently broken or missing, and what a successful fix +would accomplish. Naming the part of the codebase it affects is helpful context.] +When a user submits a review, the agent reviews their submission, generates the result of the review tools, and caches the agent state. However, if a user updates their portfolio and requests a second review, the agent reuses the same results rather than compute a new one. This means that rather than review the newly updated portfolio, the user gets feedback on their previous submission. + +**Branch name:** fix/43-agent-state-not-cleared + +**Setup confirmation:** [x] App runs locally at localhost:5173 + +**Cohort ledger:** [x] Issue added to cohort ledger + +# "Is this right for me?" Checklist + +## Part 1 — Understanding the issue +### Can I explain what this issue is asking for in my own words? +Yes, I believe my 3-5 sentences were suffice enough to explain to anyone about what the problem is, and give them a picture of what should be solved. + +### Do I understand which part of the app is affected? +labels: "agent" +relevant files: "agent/memory/session_store.py" + +These were given in the issue tab, and gives a general overview on what files I should check and understand before making any changes. + +### Do I understand what "done" looks like? + +Currently, when the user submits a review and gets feedback on it, the session is saved. However, when the user submits a second review, the agent reuses the same feedback from the first review. + +For this issue to be considered "resolved", the agent must be able to reset their session for each independent review under the same user. What this means is that for the first review, the agent creates a session state with its responses. For the second review, the agent must be able to clear the previous session state and generate a new response for the updated review. + +## Part 2 — Tier Fit +Because this is my first open source contribution, I'm choosing Tier 1 to learn the basics and understand the whole workflow before moving on. + +## Part 3 — Codebase Readiness +### Can I find the relevant code? +The relevant files given was "agent/memory/session_store.py". Given here, I retraced the code to see what files called this function, and found "orchestrator.py" utilized ".get" and '.update". I utilized Claude to help me find where these function methods were used. +### Do I understand the surrounding code well enough to change it safely? +Having traced the relevant files to where it is used throughout the project, I believe I understand the surrounding code well enough to make sure that any changes made only impact the files that used the function. + +### Have I read the relevant test file? +I took a look at "tests/unit/", but were unable to find a test file for my issue. I'll most likely produce a test file using Claude as I proceed with the isuse to make sure that my changes fixed the issue or not. + +## Part 4 — Scope and Time +### How many others are already working on this issue? +There are 23 listed on the ledger, making me the 24th. Since it's my first open source contribution, I'm fine with this count. +### Is the scope realistic for Weeks 8–9? +Since its Tier 1, I believe it's feasible to complete before Week 9. +### Are there any blockers or dependencies? +No open blockers or dependencies. From 85da34fd91114fdfcd3119f860b33a5dbf5cd8a5 Mon Sep 17 00:00:00 2001 From: ec6862 Date: Wed, 29 Jul 2026 01:24:02 -0400 Subject: [PATCH 3/8] test(agent): reproduce the session-state leak --- .gitignore | 4 + JOURNAL.md | 19 +++- PLAN.md | 35 ++++++ agent/error_handling.py | 66 ++++++++---- agent/memory/context_manager.py | 11 +- agent/memory/session_store.py | 8 +- agent/orchestrator.py | 76 ++++++------- tests/unit/test_orchestrator_session_reset.py | 102 ++++++++++++++++++ 8 files changed, 252 insertions(+), 69 deletions(-) create mode 100644 PLAN.md create mode 100644 tests/unit/test_orchestrator_session_reset.py diff --git a/.gitignore b/.gitignore index 68015fded..4b824362d 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,7 @@ htmlcov/ # Build artifacts *.pyc + +# add-on extensions +.specstory +.cursorindexingignore \ No newline at end of file diff --git a/JOURNAL.md b/JOURNAL.md index 73dc7801c..4c6a31b35 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -1,4 +1,4 @@ -## Week 7 — Issue selection +# Week 7 — Issue selection **Issue link:** https://github.com/ascherj/pathreview/issues/43 @@ -17,7 +17,7 @@ When a user submits a review, the agent reviews their submission, generates the **Cohort ledger:** [x] Issue added to cohort ledger -# "Is this right for me?" Checklist +## "Is this right for me?" Checklist ## Part 1 — Understanding the issue ### Can I explain what this issue is asking for in my own words? @@ -54,3 +54,18 @@ There are 23 listed on the ledger, making me the 24th. Since it's my first open Since its Tier 1, I believe it's feasible to complete before Week 9. ### Are there any blockers or dependencies? No open blockers or dependencies. + +# Week 8 — Reproduction & solution planning + +**Reproduction commit link:** [link to commit documenting the reproduced issue] + +**Reproduction summary:** + +I utilized Claude to help regenerate the issue. I based my PLAN.md and JOURNAL.md, and asked it to reproduce what the issue can occur. Afterwards, I asked it to store it in a file, in which it is now located in "/tests/unit/test_orchestrator_session_reset.py". + +**PLAN.md link:** [link to PLAN.md in your fork] + +**Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded] + +**Blockers or open questions:** +[Anything you're still uncertain about going into Week 9, or leave blank] \ No newline at end of file diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 000000000..2a697eb0a --- /dev/null +++ b/PLAN.md @@ -0,0 +1,35 @@ +## Solution plan + +**Issue:** Agent session state is not cleared between reviews for the same user #43, https://github.com/ascherj/pathreview/issues/43 + +### Understand + +The root cause of the issue is not deleting the previous result, and instead continue using that result with the newly created response. + +Expected: User submits a review, gets feedback that is stored. The next submission, the user submits a new review, the agent checks the new review, and gives a new response, storing the new response over the previous one. + +### Map + +1. agent/memory/session_store.py +2. agent/orchestrator.py + +### Plan + +1. Review the issue on Github, understanding what session_store.py provides and what each function does. +2. Backtrace and find instances where session_store.py is used. +3. Check the usage, noting how its used. Check to make sure the usage follows the guidelines. If it doesn't, note what is wrong with it. +4. After finding the mistake, review what the original purpose is, how to implement it, and draft the corrected version. +5. Test the code, if it works, create a PR. Else, go back to step 4. + +### Inputs & outputs +What does your fix take as input? What should it produce or change? +The input takes in "profile_id" and "profile_data", and returns a dictionary containing "profile_id," "tool_results," and "cached_results." The fix should make sure that the output only returns the current review's results. + +### Risks & unknowns +What could go wrong? What are you still unsure about? +Some unknowns raised is when "profile_id" is not unique per user/review. If they are truly unique, would deleting mid-run lose data if two reviews run concurrently on the same profile? +### Edge cases +What inputs or states should your fix handle gracefully? +First review should have 0 prior state, and review 2 should have fewer tools than review 1. Alongside this, we should be able to deal with an empty "profile_data". Reproducing these edge cases should prove the bug. \ No newline at end of file diff --git a/agent/error_handling.py b/agent/error_handling.py index 438dd409e..24ac561d6 100644 --- a/agent/error_handling.py +++ b/agent/error_handling.py @@ -1,14 +1,18 @@ """Retry logic with exponential backoff.""" -import time import functools +import time +from collections.abc import Callable +from typing import Any + import structlog logger = structlog.get_logger() -def retry_with_backoff(max_retries: int = 3, backoff_factor: float = 2.0, - exceptions: tuple = (Exception,)): +def retry_with_backoff( + max_retries: int = 3, backoff_factor: float = 2.0, exceptions: tuple = (Exception,) +) -> Callable: """Decorator for retry logic with exponential backoff. Args: @@ -19,9 +23,10 @@ def retry_with_backoff(max_retries: int = 3, backoff_factor: float = 2.0, Returns: Decorated function """ - def decorator(func): + + def decorator(func: Callable) -> Callable: @functools.wraps(func) - def wrapper(*args, **kwargs): + def wrapper(*args: Any, **kwargs: Any) -> Any: attempt = 0 last_exception = None @@ -36,26 +41,37 @@ def wrapper(*args, **kwargs): if attempt < max_retries: wait_time = backoff_factor ** (attempt - 1) - logger.warning("retry_attempt", func=func.__name__, - attempt=attempt, max_retries=max_retries, - wait_seconds=wait_time, error=str(e)) + logger.warning( + "retry_attempt", + func=func.__name__, + attempt=attempt, + max_retries=max_retries, + wait_seconds=wait_time, + error=str(e), + ) time.sleep(wait_time) else: - logger.error("retry_exhausted", func=func.__name__, - max_retries=max_retries, error=str(e)) + logger.error( + "retry_exhausted", + func=func.__name__, + max_retries=max_retries, + error=str(e), + ) if last_exception: raise last_exception return wrapper + return decorator class RetryContext: """Context manager for retry logic with exponential backoff.""" - def __init__(self, max_retries: int = 3, backoff_factor: float = 2.0, - exceptions: tuple = (Exception,)): + def __init__( + self, max_retries: int = 3, backoff_factor: float = 2.0, exceptions: tuple = (Exception,) + ): """Initialize retry context. Args: @@ -67,13 +83,15 @@ def __init__(self, max_retries: int = 3, backoff_factor: float = 2.0, self.backoff_factor = backoff_factor self.exceptions = exceptions self.attempt = 0 - self.last_exception = None + self.last_exception: BaseException | None = None - def __enter__(self): + def __enter__(self) -> "RetryContext": """Enter context.""" return self - def __exit__(self, exc_type, exc_val, exc_tb): + def __exit__( + self, exc_type: type | None, exc_val: BaseException | None, exc_tb: Any | None + ) -> bool: """Exit context and handle retries.""" if exc_type is None: return False @@ -86,12 +104,20 @@ def __exit__(self, exc_type, exc_val, exc_tb): if self.attempt < self.max_retries: wait_time = self.backoff_factor ** (self.attempt - 1) - logger.warning("retry_context_attempt", attempt=self.attempt, - max_retries=self.max_retries, wait_seconds=wait_time, - error=str(exc_val)) + logger.warning( + "retry_context_attempt", + attempt=self.attempt, + max_retries=self.max_retries, + wait_seconds=wait_time, + error=str(exc_val), + ) time.sleep(wait_time) return True # Suppress exception and retry - logger.error("retry_context_exhausted", attempt=self.attempt, - max_retries=self.max_retries, error=str(exc_val)) + logger.error( + "retry_context_exhausted", + attempt=self.attempt, + max_retries=self.max_retries, + error=str(exc_val), + ) return False # Re-raise exception diff --git a/agent/memory/context_manager.py b/agent/memory/context_manager.py index 5cebe3e58..c87931fbf 100644 --- a/agent/memory/context_manager.py +++ b/agent/memory/context_manager.py @@ -2,6 +2,8 @@ import hashlib import json +from typing import Any + import structlog logger = structlog.get_logger() @@ -10,12 +12,11 @@ class ContextManager: """In-memory context manager for within-session memoization.""" - def __init__(self): + def __init__(self) -> None: """Initialize context manager.""" - self.results = {} + self.results: dict = {} - def store_tool_result(self, tool_name: str, input_hash: str, - result) -> None: + def store_tool_result(self, tool_name: str, input_hash: str, result: Any) -> None: """Store tool execution result. Args: @@ -27,7 +28,7 @@ def store_tool_result(self, tool_name: str, input_hash: str, self.results[key] = result logger.info("tool_result_stored", tool=tool_name, key=key) - def get_tool_result(self, tool_name: str, input_hash: str): + def get_tool_result(self, tool_name: str, input_hash: str) -> Any | None: """Get cached tool result. Args: diff --git a/agent/memory/session_store.py b/agent/memory/session_store.py index c10d296ed..0183a759d 100644 --- a/agent/memory/session_store.py +++ b/agent/memory/session_store.py @@ -1,9 +1,9 @@ """Redis-backed session store.""" -import redis import json + +import redis import structlog -from typing import Optional logger = structlog.get_logger() @@ -19,7 +19,7 @@ def __init__(self, redis_client: redis.Redis): """ self.redis = redis_client - def get(self, session_id: str) -> Optional[dict]: + def get(self, session_id: str) -> dict | None: """Get session data. Args: @@ -36,7 +36,7 @@ def get(self, session_id: str) -> Optional[dict]: logger.info("session_not_found", session_id=session_id) return None - parsed = json.loads(data) + parsed: dict = json.loads(data) logger.info("session_retrieved", session_id=session_id) return parsed diff --git a/agent/orchestrator.py b/agent/orchestrator.py index 641b42833..69f39b278 100644 --- a/agent/orchestrator.py +++ b/agent/orchestrator.py @@ -1,12 +1,13 @@ """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() @@ -14,8 +15,9 @@ 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: @@ -53,7 +55,7 @@ def run(self, profile_id: str, profile_data: dict) -> dict: 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) @@ -66,13 +68,12 @@ def run(self, profile_id: str, profile_data: dict) -> dict: session_state.update(results) self.session_store.set(profile_id, session_state) - 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,9 @@ 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: @@ -189,7 +189,7 @@ def _execute_with_timeout(self, tool, tool_input: dict, timeout: Optional[float] timeout = timeout or self.tool_timeout @retry_with_backoff(max_retries=2, backoff_factor=1.5) - def _execute(): + def _execute() -> Any: return tool.execute(tool_input) start = time.time() diff --git a/tests/unit/test_orchestrator_session_reset.py b/tests/unit/test_orchestrator_session_reset.py new file mode 100644 index 000000000..64611eb27 --- /dev/null +++ b/tests/unit/test_orchestrator_session_reset.py @@ -0,0 +1,102 @@ +"""Reproduction / regression test for issue #43. + +Agent session state is not cleared between reviews for the same user. +https://github.com/ascherj/pathreview/issues/43 + +Orchestrator.run() loads the previous session state and then does +`session_state.update(results)`, merging new results ONTO the old ones instead +of replacing them. So a tool that ran in an earlier review but not in a later +one leaves its stale output behind in the stored session. + +This test currently FAILS (documents the bug). It should PASS once the +orchestrator clears prior state before persisting the current review. +""" + +from typing import cast + +import pytest +import redis + +from agent.memory.session_store import SessionStore +from agent.orchestrator import Orchestrator +from agent.tools.base import BaseTool, ToolResult + + +class FakeRedis: + """Minimal in-memory stand-in for the redis client SessionStore uses.""" + + def __init__(self) -> None: + self.store: dict[str, str] = {} + + def get(self, key: str) -> str | None: + return self.store.get(key) + + def setex(self, key: str, ttl_seconds: int, value: str) -> None: + self.store[key] = value + + def delete(self, key: str) -> None: + self.store.pop(key, None) + + +class EchoTool(BaseTool): + """Stub tool that echoes its input so we can tell which review produced it.""" + + def __init__(self, name: str) -> None: + self.name = name + self.description = name + + def execute(self, input_data: dict) -> ToolResult: + return ToolResult(success=True, data={"tool": self.name, "input": input_data}) + + +def _make_orchestrator(session_store: SessionStore) -> Orchestrator: + tools = { + "github_tool": EchoTool("github_tool"), + "tech_detector": EchoTool("tech_detector"), + "readme_scorer": EchoTool("readme_scorer"), + "skill_extractor": EchoTool("skill_extractor"), + "market_analyzer": EchoTool("market_analyzer"), + } + # A fresh Orchestrator per call models two independent review requests from + # the same user, sharing one Redis-backed session store. + return Orchestrator(tools=tools, session_store=session_store) + + +def test_session_state_cleared_between_reviews() -> None: + """A later review must not retain tool results from an earlier one.""" + # FakeRedis implements the subset of the redis client SessionStore uses. + store = SessionStore(cast(redis.Redis, FakeRedis())) + profile_id = "user-123" + + # Review 1: portfolio WITH a resume -> skill_extractor runs and is stored. + profile_v1 = { + "github_username": "janedoe", + "projects": [{"github_repo": "weather-app"}], + "readme_content": "# Weather App (v1)", + "resume_text": "Python, FastAPI", + } + _make_orchestrator(store).run(profile_id, profile_v1) + assert "skill_extractor" in (store.get(profile_id) or {}) # sanity: ran in review 1 + + # Review 2: user updated the portfolio and REMOVED the resume, so + # skill_extractor is not part of this review's plan. + profile_v2 = { + "github_username": "janedoe", + "projects": [{"github_repo": "weather-app"}], + "readme_content": "# Weather App (v2 - IMPROVED)", + } + _make_orchestrator(store).run(profile_id, profile_v2) + + stored = store.get(profile_id) + assert stored is not None + + # The stored session should reflect ONLY review 2's tools. Today it still + # contains review 1's skill_extractor -> this assertion fails (the bug). + assert "skill_extractor" not in stored, ( + "Stale 'skill_extractor' from review 1 leaked into the session after " + "review 2 (session state not cleared between reviews)." + ) + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__, "-v"])) From 731bb97007742fa427e80e069276f63b3f7d0728 Mon Sep 17 00:00:00 2001 From: Eric Chen <60475399+ec6862@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:26:24 -0400 Subject: [PATCH 4/8] docs: fill in Week 8 reproduction and PLAN links --- JOURNAL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index 4c6a31b35..e652fed32 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -57,15 +57,15 @@ No open blockers or dependencies. # Week 8 — Reproduction & solution planning -**Reproduction commit link:** [link to commit documenting the reproduced issue] +**Reproduction commit link:** https://github.com/AI201-Applications-of-AI-Engineering/pathreview/commit/08fd74e1ec17692fc04d7f115b6d44ed3b3435e8 **Reproduction summary:** I utilized Claude to help regenerate the issue. I based my PLAN.md and JOURNAL.md, and asked it to reproduce what the issue can occur. Afterwards, I asked it to store it in a file, in which it is now located in "/tests/unit/test_orchestrator_session_reset.py". -**PLAN.md link:** [link to PLAN.md in your fork] +**PLAN.md link:** https://github.com/AI201-Applications-of-AI-Engineering/pathreview/blob/fix/43-agent-state-not-cleared/PLAN.md **Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded] **Blockers or open questions:** -[Anything you're still uncertain about going into Week 9, or leave blank] \ No newline at end of file +[Anything you're still uncertain about going into Week 9, or leave blank] From 721c2b14ff2d215043956d2053be2e644c8ebb4b Mon Sep 17 00:00:00 2001 From: ec6862 Date: Thu, 6 Aug 2026 19:54:00 -0400 Subject: [PATCH 5/8] fix(agent): cleared session state between reviews --- agent/orchestrator.py | 8 +------- tests/unit/test_orchestrator_session_reset.py | 9 ++++++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/agent/orchestrator.py b/agent/orchestrator.py index 69f39b278..f43d796ba 100644 --- a/agent/orchestrator.py +++ b/agent/orchestrator.py @@ -45,11 +45,6 @@ def run(self, profile_id: str, profile_data: dict) -> dict: # 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: @@ -65,8 +60,7 @@ def run(self, profile_id: str, profile_data: dict) -> dict: # Persist state 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)) diff --git a/tests/unit/test_orchestrator_session_reset.py b/tests/unit/test_orchestrator_session_reset.py index 64611eb27..6b56781f2 100644 --- a/tests/unit/test_orchestrator_session_reset.py +++ b/tests/unit/test_orchestrator_session_reset.py @@ -12,15 +12,17 @@ orchestrator clears prior state before persisting the current review. """ -from typing import cast +from typing import TYPE_CHECKING, cast import pytest -import redis from agent.memory.session_store import SessionStore from agent.orchestrator import Orchestrator from agent.tools.base import BaseTool, ToolResult +if TYPE_CHECKING: + import redis + class FakeRedis: """Minimal in-memory stand-in for the redis client SessionStore uses.""" @@ -62,10 +64,11 @@ def _make_orchestrator(session_store: SessionStore) -> Orchestrator: return Orchestrator(tools=tools, session_store=session_store) +@pytest.mark.unit def test_session_state_cleared_between_reviews() -> None: """A later review must not retain tool results from an earlier one.""" # FakeRedis implements the subset of the redis client SessionStore uses. - store = SessionStore(cast(redis.Redis, FakeRedis())) + store = SessionStore(cast("redis.Redis", FakeRedis())) profile_id = "user-123" # Review 1: portfolio WITH a resume -> skill_extractor runs and is stored. From 0931d05b334131d64da16e3ce7cd01ba91764bdb Mon Sep 17 00:00:00 2001 From: ec6862 Date: Sat, 8 Aug 2026 20:08:32 -0400 Subject: [PATCH 6/8] docs: filled in Week 9 check-in 1 --- JOURNAL.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/JOURNAL.md b/JOURNAL.md index e652fed32..3569cacb4 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -69,3 +69,35 @@ I utilized Claude to help regenerate the issue. I based my PLAN.md and JOURNAL.m **Blockers or open questions:** [Anything you're still uncertain about going into Week 9, or leave blank] + +# Week 9 — Solution building & PR submission + +## Check-in 1 (mid-week) + +**Current progress:** +[What have you implemented so far? Which sub-tasks from PLAN.md are done?] +I've completed the fix, having tested with the sample test file that I've built. To make sure there were no issues, I ran the file before and after my change to make sure there were noticeable changes. + +**Next steps:** +[What are you working on for the rest of the week?] +Addressing the PR, seeing if I can improve on it, and if I am missing anything, and practice more of it. +**Blockers:** +[Anything slowing you down? Or leave blank.] +I am submitting this a week later than the actual due date, so I apologize if I'm not able to get feedback for myself. Thank you for all the help this year. +--- + +## Check-in 2 (end of week) + +**PR link:** [link to your submitted pull request] + +**Branch:** [the branch name you worked on, e.g. `fix/123-short-description`] + +**What you built:** +[1–3 sentences summarizing what your fix does and how it works] + +**Tests added or updated:** +[Which test files did you touch? What do they cover?] + +**Self-review confirmation:** [ ] make check passes [ ] make test-unit passes + +**Draft PR feedback received from:** [name or Slack handle, or "none"] \ No newline at end of file From 23ea876b9482f8486432eae6c5d8402d497f0066 Mon Sep 17 00:00:00 2001 From: ec6862 Date: Sat, 8 Aug 2026 20:59:01 -0400 Subject: [PATCH 7/8] docs: filled in Week 9 check-in 2 with PR opened --- JOURNAL.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index 3569cacb4..a28593c15 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -88,16 +88,20 @@ I am submitting this a week later than the actual due date, so I apologize if I' ## Check-in 2 (end of week) -**PR link:** [link to your submitted pull request] +**PR link:** https://github.com/ascherj/pathreview/pull/1008 -**Branch:** [the branch name you worked on, e.g. `fix/123-short-description`] +**Branch:** + +fix/43-agent-state-not-cleared **What you built:** -[1–3 sentences summarizing what your fix does and how it works] +The original bug was that when a user requests a review, they are able to receive feedback on it. However, if the user submits a revised review, the stored session still contained the results from the first review instead of producing a clean fresh analysis of the new submission. What I've done is completely remove the loading of the previous state, which caused a buggy merge, and directly store the results of the new review within the state by overwriting what was previously there. **Tests added or updated:** [Which test files did you touch? What do they cover?] +Because there was no test file made for this bug, I created a sample test file, named "test_orchestrator_session_reset.py". This test file tested the leak existed before the fix, and what it outputted after the fix by having one user submitting two reviews against a shared store. The test would fail if the orchestrator merged review 2 onto review 1's state, and would pass if it replaced the state. **Self-review confirmation:** [ ] make check passes [ ] make test-unit passes +NOTE: The errors that existed before the change and after the change remained, but the changes introduced zero new failures. Because this error did not have its own test as part of the test-unit, I had to manually add the test into it to make sure that it passed. **Draft PR feedback received from:** [name or Slack handle, or "none"] \ No newline at end of file From ad0cc4babff5358f9ae551fa035ac06f9f1a291e Mon Sep 17 00:00:00 2001 From: ec6862 Date: Mon, 10 Aug 2026 00:21:46 -0400 Subject: [PATCH 8/8] docs: filled in Week 10, iteration + reflection; no reviews --- JOURNAL.md | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/JOURNAL.md b/JOURNAL.md index a28593c15..5ecf74925 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -104,4 +104,47 @@ Because there was no test file made for this bug, I created a sample test file, **Self-review confirmation:** [ ] make check passes [ ] make test-unit passes NOTE: The errors that existed before the change and after the change remained, but the changes introduced zero new failures. Because this error did not have its own test as part of the test-unit, I had to manually add the test into it to make sure that it passed. -**Draft PR feedback received from:** [name or Slack handle, or "none"] \ No newline at end of file +**Draft PR feedback received from:** [name or Slack handle, or "none"] + +# Week 10 — Iteration & reflection + +## Reviewer feedback + +**Feedback received:** [ ] Yes [x] No — still awaiting review + +**Summary of feedback:** +[What did reviewers comment on? Or note that no review came in.] +No reviews yet. + +**How you responded:** +[What changes did you make, or what did you reply? If no feedback, +leave blank.] + +--- + +## Reflection + +**What was harder than you expected?** +[Be specific — what part of the process, codebase, or workflow +surprised you?] +Having this as my first PR ever, understanding the process and successfully following the guidelines was probably the hardest part for me. Because I was new to this process, I didn't know where to start or where to look at. Though I was able to use AI to try to understand the project that I was working with, I still didn't know exactly where to start. Because of this, having AI actually saved so much time, directing me to where the bug occurred. Having figured this out, the next step was understanding what the code did, and backtracking from here to figure out where it starts and then process it up. Once I was able to understand (and having Claude to help me out with examples and clarifications), I was able to find the issue by isolating predicted/actual results and having Claude help me print the actual errors, and go from there. + +**What did you learn about working in a large codebase?** +[What's different about contributing to someone else's production code +vs. building your own project?] +Working in a large codebase gave me an experience on how other SWEs work. Not everything is start from scratch; many times, it starts with an older production base, and it's with this project that I learned the fundamentals of SWE. Having to first understand the problem, understanding the codebase, and then connecting the two to find the issue. While this is my simplified version of what SWEs do, I felt like this was a very good step to understanding how to contribute to future bigger projects, and I'll be able to learn more as I go. + +**How did AI tools help — and where did they fall short?** +[Where was AI assistance most useful this module? Where did you need +to go beyond what AI could give you?] +AI assistance was best for me in understanding where to start, what the files that were associated with the issues did, and where the problem occurred. Having the baseline of the problems, I was able to study each file and understand what they did, vs what they are suppose to do. +Though I had AI to help me understand and isolate the bug, I didn't use it to write my statements. I wanted the readers to understand what I wrote, and understand clearly so that when future reviewers read this issue, they are able to understand what I wanted to do, and what the expected results were. Because AI writes very robotic, I figure writing everything myself could allow reviewers to know that every statement was written to be understood at layman's terms. + +**What would you do differently if you started over?** +[Issue selection, planning, implementation, or process — anything +you'd change?] +Doing everything in one go. Starting from the issue selection to planning to implementation, I want to be able to understand everything in the beginning before trying to figure out where the error occurred. + +**What are you most proud of from this module?** +[One thing — it doesn't have to be the PR itself.] +Being able to know what to do from start to finish. Even if there was one thing wrong or something that I could've done better, this was a learning experience for me and any sort of success/failure helps me grow. \ No newline at end of file