test: add loop helper parsing tests#26
Conversation
📝 WalkthroughWalkthroughA new test module Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/test_loop_helpers.py (1)
20-109: Optional: usepytest.mark.parametrizeto reduce repetition.This test module is already solid; parametrizing similar cases would make future additions easier to maintain.
♻️ Example refactor pattern
+@pytest.mark.unit +@pytest.mark.parametrize( + ("content", "expected_gap", "expected_context"), + [ + ("GAP: what is X?\nCONTEXT: figuring out X\nREMEMBER", "what is X?", "figuring out X"), + ("CONTEXT: still working through it\nREMEMBER", "", "still working through it"), + ("GAP: what is Y?\nREMEMBER", "what is Y?", ""), + ("Some unrelated reasoning text.\nNo markers here.\nDONE", "", ""), + ("\n GAP: what is Z? \n\n CONTEXT:\treasoning about Z\t\n\nREMEMBER\n", "what is Z?", "reasoning about Z"), + ], +) +def test_extract_gap_cases(loop: Loop, content: str, expected_gap: str, expected_context: str) -> None: + gap, context = loop._extract_gap(content) + assert gap == expected_gap + assert context == expected_context🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_loop_helpers.py` around lines 20 - 109, Several tests in the test_extract_gap_* and test_strip_marker_* groups repeat the same structure; refactor them to parametrize inputs and expected outputs using pytest.mark.parametrize to reduce duplication. Replace the multiple test_extract_gap_* functions with a single parametrized test that calls loop._extract_gap(content) for tuples of (content, expected_gap, expected_context) and similarly replace the strip_marker tests with a single parametrized test that calls loop._strip_marker(content, "DONE") for tuples of (content, expected_result), keeping unique edge-case names as ids; ensure you reference the existing helper methods _extract_gap and _strip_marker and preserve all current cases and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/test_loop_helpers.py`:
- Around line 20-109: Several tests in the test_extract_gap_* and
test_strip_marker_* groups repeat the same structure; refactor them to
parametrize inputs and expected outputs using pytest.mark.parametrize to reduce
duplication. Replace the multiple test_extract_gap_* functions with a single
parametrized test that calls loop._extract_gap(content) for tuples of (content,
expected_gap, expected_context) and similarly replace the strip_marker tests
with a single parametrized test that calls loop._strip_marker(content, "DONE")
for tuples of (content, expected_result), keeping unique edge-case names as ids;
ensure you reference the existing helper methods _extract_gap and _strip_marker
and preserve all current cases and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a1274e2f-0b6c-4607-b2b9-efd11e7b573f
📒 Files selected for processing (1)
tests/unit/test_loop_helpers.py
AlexanderGardiner
left a comment
There was a problem hiding this comment.
Looks good! I'll go ahead and merge.
Pull Request
Summary
Adds deterministic unit tests for the helper parsing behavior in
src/anchor/loop.py. This PR covers_extract_gap()and_strip_marker()with focused offline tests so regressions in loop protocol parsing are easier to catch and debug.Linked context
Closes #13
PR Size
PR Type
promptlabel required; eval results required below)Context / Motivation
These helpers are small, but they are responsible for parsing
GAP:/CONTEXT:content emitted by the loop protocol and for stripping terminal markers from returned text. Adding focused tests helps catch regressions without expanding into fullAnchor.run()integration coverage.Changes
Added
tests/unit/test_loop_helpers.pywith deterministic unit tests for_extract_gap()and_strip_marker(), covering standard parsing, missing markers, whitespace handling, and terminal marker stripping behavior.How to test
uv run pytest -m "not eval" tests/unit/test_loop_helpers.py -vuv run pytest -m "not eval"uv run pre-commit run --all-filesChecklist
Notes for reviewers
This PR stays intentionally narrow and only adds helper-level unit tests. No production code or protocol behavior was changed.
Summary by CodeRabbit
Release Notes