tests/test_pr_branch_mapping.py (3)

226-231: Unused variable msg.

Ruff RUF059 flags unpacked variable msg as unused. Prefix with underscore.

Proposed fix

-        is_ok, msg = validate_branch_pr_consistency(
+        is_ok, _msg = validate_branch_pr_consistency(
             sample_mapping, current_branch="feat/add-widget"
         )

🤖 Prompt for AI Agents

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

In `@tests/test_pr_branch_mapping.py` around lines 226 - 231, The test unpacks
validate_branch_pr_consistency into is_ok, msg but never uses msg which triggers
RUF059; change the unpack to use a prefixed-underscore name (e.g., is_ok, _msg =
validate_branch_pr_consistency(...) or is_ok, _ =
validate_branch_pr_consistency(...)) in tests/test_pr_branch_mapping.py where
validate_branch_pr_consistency is called, and ensure no subsequent references to
msg remain.

---

334-345: Unused capsys parameter.

Ruff ARG002 flags capsys as unused. Either remove it or use it to verify stdout content.

Option A: Remove unused parameter

-    def test_query_by_pr(self, tmp_path: Path, capsys: object) -> None:
+    def test_query_by_pr(self, tmp_path: Path) -> None:

Option B: Actually verify output

-    def test_query_by_pr(self, tmp_path: Path, capsys: object) -> None:
+    def test_query_by_pr(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None:
         mapping = PRBranchMapping()
         add_mapping(mapping, 42, "feat/test")
         save_mapping(tmp_path, mapping)
 
         exit_code = main([
             "--project-root", str(tmp_path),
             "query", "--pr", "42",
         ])
 
         assert exit_code == 0
+        captured = capsys.readouterr()
+        assert "feat/test" in captured.out

🤖 Prompt for AI Agents

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

In `@tests/test_pr_branch_mapping.py` around lines 334 - 345, The test function
test_query_by_pr currently declares an unused capsys parameter causing Ruff
ARG002; either remove the capsys parameter from test_query_by_pr (and leave the
rest as-is using PRBranchMapping, add_mapping, save_mapping and main) or, if you
intend to validate CLI output, use capsys to capture stdout after calling main
(e.g., call capsys.readouterr() and assert on its .out) to verify the expected
output for the "query --pr 42" invocation; update assertions accordingly and
keep the rest of the setup (PRBranchMapping, add_mapping, save_mapping) intact.

---

221-221: Repeated import inside test methods.

CurrentSession is imported inside 4 test methods (lines 221, 236, 253, 310). Move to module-level imports at line 15-27 for consistency.

Proposed fix at module level

 from scripts.pr_branch_mapping import (
+    CurrentSession,
     MEMORY_RELATIVE_PATH,
     PRBranchEntry,
     PRBranchMapping,

Then remove the from scripts.pr_branch_mapping import CurrentSession lines from inside test methods.

🤖 Prompt for AI Agents

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

In `@tests/test_pr_branch_mapping.py` at line 221, Multiple tests import
CurrentSession inside their bodies; move the import to the module-level with the
other imports at the top of the test file and remove the four in-function
imports. Specifically, add "from scripts.pr_branch_mapping import
CurrentSession" to the top import block (with the other test imports) and delete
the in-test occurrences of that import in the methods that currently re-import
CurrentSession.

scripts/pr_branch_mapping.py (4)

27-28: Direct file I/O bypasses MemoriesManager.

Per external tools context, repository docs describe MemoriesManager as the canonical API for project-local memories under .serena/memories/. Direct writes bypass name normalization, file-size limits, and audit semantics.

Consider refactoring to use MemoriesManager in a follow-up if consistency with other memory tools is required.

🤖 Prompt for AI Agents

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

In `@scripts/pr_branch_mapping.py` around lines 27 - 28, The code creates
MEMORY_FILENAME and MEMORY_RELATIVE_PATH constants that point directly at
".serena/memories/...", which bypasses the project's MemoriesManager; replace
direct file path usage by obtaining the canonical memory path or handle from the
MemoriesManager API (e.g., call MemoriesManager.get_memory_path(name) or
MemoriesManager.open/create with MEMORY_FILENAME) so name normalization, size
limits and audit semantics are applied instead of hardcoding
MEMORY_RELATIVE_PATH; update any code that referenced MEMORY_RELATIVE_PATH to
use the MemoriesManager methods (retain MEMORY_FILENAME only as a logical name
if needed).

---

87-89: Simplify with dict.get().

Ruff RUF019 flags the redundant key check. dict.get() handles missing keys cleanly.

Proposed fix

     current = None
-    if "current_session" in data and data["current_session"]:
-        current = CurrentSession(**data["current_session"])
+    session_data = data.get("current_session")
+    if session_data:
+        current = CurrentSession(**session_data)

🤖 Prompt for AI Agents

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

In `@scripts/pr_branch_mapping.py` around lines 87 - 89, The code redundantly
checks "current_session" in data before instantiating CurrentSession; replace
that pattern with dict.get() to satisfy Ruff RUF019 by retrieving the value via
data.get("current_session") and only instantiate CurrentSession(**...) when the
returned value is truthy (e.g., use a conditional expression or an if on value)
so the variable current is None when there is no session and becomes
CurrentSession(**value) when present.

---

385-387: Blind exception catch loses stack trace.

Ruff BLE001 flags this. For a CLI tool, catching all exceptions is acceptable to prevent crashes. But the current message swallows the traceback, making debugging harder.

Consider logging the exception type or using logging.exception() when verbose mode is added.

Proposed fix

     except Exception as exc:
-        print(f"FATAL: {exc}", file=sys.stderr)
+        print(f"FATAL: {type(exc).__name__}: {exc}", file=sys.stderr)
         return 2

🤖 Prompt for AI Agents

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

In `@scripts/pr_branch_mapping.py` around lines 385 - 387, The broad except block
that prints only the exception message (in the except Exception as exc: ...
return 2) loses the traceback; update that handler to emit the full traceback
and exception type so failures are debuggable (e.g., use logging.exception(...)
or traceback.print_exc(file=sys.stderr) instead of print(f"FATAL: {exc}", ...)).
Locate the top-level exception handler in the script (the except Exception as
exc: block that returns 2) and replace the single-line print with a call that
logs the traceback and message; if a verbose/logging mode exists or is added,
prefer logging.exception for consistent logging.

---

109-148: Function mutates input and returns it.

add_mapping modifies mapping in place and also returns it. This dual pattern can confuse callers about ownership—some may expect a pure function returning a new object, others may expect a void mutator.

Current usage appears consistent, but consider documenting this explicitly in the docstring or picking one pattern.

🤖 Prompt for AI Agents

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

In `@scripts/pr_branch_mapping.py` around lines 109 - 148, The add_mapping
function mutates the provided PRBranchMapping in place and also returns it,
which can confuse callers; update the add_mapping docstring to explicitly state
that it modifies the supplied mapping object (PRBranchMapping) in-place and
returns the same modified instance, and reference the behavior of fields
updated/added (mapping.mappings appends PRBranchEntry, existing entry fields
branch_name/last_session are updated, and mapping.current_session is replaced
with a CurrentSession) so callers know it is not a pure copy-producing function
(alternatively, if you prefer an immutable API, replace in-place updates in
add_mapping/_find_entry with code that clones mapping and returns a new
PRBranchMapping — choose one approach and make docstring/signature consistent).