refactor: reduce cognitive complexity across all modules (ISS-280) by NeuralEmpowerment · Pull Request #117 · AgentParadise/agentic-primitives

NeuralEmpowerment · 2026-03-24T00:57:34Z

Summary

Refactor 18 functions to meet CC≤25 / LOC≤100 fitness thresholds
Add shared BaseProvider._read_stream_lines() eliminating duplication between docker and local providers
19 new regression tests for all extracted helpers (159 total, up from 140 baseline)

Refactored modules

Module	Functions	Strategy
`providers/docker.py`	`stream`, `execute`	Extract `_build_docker_exec_cmd`, `_run_exec`, delegate to shared stream helper
`providers/local.py`	`stream`, `execute`	Extract `_build_exec_env`, `_resolve_work_dir`, delegate to shared stream helper
`providers/base.py`	(new)	Add `_read_stream_lines`, `_check_stream_timeout`, `_terminate_process`
`event_parser.py`	`parse_line`, `_handle_assistant`	Extract `_try_parse_json`, `_handle_recording_metadata`, `_extract_token_usage`, `_handle_tool_use_item`
`config.py`	`resolve_plugin_env`	Extract `_load_plugin_manifest`, `_resolve_single_env_var`, `_apply_env_var`
`retry.py`	`retry_async`	Extract `_should_retry`, `_calculate_delay`
`formatters.py`	`format`	Extract `_format_header`, `_format_extras`
Validators, hooks, env scripts	Various	Mechanical extraction of inner loops/branches

Test plan

All 159 tests pass (uv run pytest tests/ -v)
No behavioral changes — pure extract-method refactoring
Parent repo fitness-check passes with zero agentic-primitives exceptions

Related: syntropic137/syntropic137#308

Extract helper functions to bring all 18 source functions below CC≤25 and LOC≤100 thresholds. Add shared _read_stream_lines() to BaseProvider (eliminates duplication between docker/local providers). Add 19 regression tests for extracted helpers.

Copilot

Pull request overview

Refactors multiple Python modules to reduce cognitive complexity/LOC, primarily by extracting shared helpers and centralizing duplicated logic (notably subprocess stream reading for providers), while adding regression tests for the extracted helpers.

Changes:

Introduces shared provider subprocess streaming helpers in BaseProvider and updates Docker/Local providers to delegate to them.
Extracts parsing/formatting/env-sync/validator subroutines into small helpers across scripts, plugins, and logging.
Adds/expands unit tests covering new internal helpers (providers + config env resolution).

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scripts/capture_recording.py	Extracts event-line processing and recording writing into helpers.
plugins/sdlc/skills/env-management/references/python/generate_env_example.py	Refactors multiline parsing and template/env merge into helper functions.
plugins/sdlc/skills/centralized-configuration/resources/python/generate_env.py	Refactors `.env` parsing/sync logic into helper functions.
plugins/sdlc/hooks/validators/security/python.py	Extracts dangerous/suspicious pattern scanning helpers.
plugins/sdlc/hooks/validators/prompt/pii.py	Splits PII scanning into focused helper functions.
plugins/sdlc/hooks/git/install.py	Extracts global/local install flows into helpers.
plugins/observability/hooks/git/install.py	Extracts global/local install flows into helpers.
lib/python/agentic_logging/agentic_logging/formatters.py	Extracts header/extra formatting into helpers for readability.
lib/python/agentic_isolation/agentic_isolation/providers/base.py	Adds shared subprocess stream reading/timeout/termination helpers.
lib/python/agentic_isolation/agentic_isolation/providers/local.py	Delegates workdir/env building + stream reading to helpers.
lib/python/agentic_isolation/agentic_isolation/providers/docker.py	Extracts docker exec cmd building and execution; delegates stream reading to shared helper.
lib/python/agentic_isolation/agentic_isolation/providers/claude_cli/event_parser.py	Extracts JSON parsing, recording metadata handling, token/tool-use processing helpers.
lib/python/agentic_isolation/agentic_isolation/config.py	Extracts plugin manifest loading + single env var resolution/apply helpers.
lib/python/agentic_isolation/agentic_isolation/retry.py	Extracts retry decision + delay/logging computation helpers.
lib/python/agentic_isolation/tests/test_providers.py	Adds tests for the new shared provider stream helpers.
lib/python/agentic_isolation/tests/test_config.py	Adds tests for extracted plugin manifest/env-resolution helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T01:01:10Z

lib/python/agentic_isolation/agentic_isolation/providers/base.py

+            proc.terminate()
+            await asyncio.wait_for(proc.wait(), timeout=5.0)
+        except (TimeoutError, ProcessLookupError):
+            proc.kill()


In _terminate_process(), if terminate/wait fails and we fall back to proc.kill(), we never await proc.wait(). That can leave a zombie process and may also leave proc.returncode as None. Consider following kill() with an awaited proc.wait() (optionally wrapped in a short wait_for) so the process is always reaped.

Suggested change

proc.kill()

try:

proc.kill()

await asyncio.wait_for(proc.wait(), timeout=5.0)

except (TimeoutError, ProcessLookupError):

# If the process still cannot be waited on, give up silently.

pass

Copilot · 2026-03-24T01:01:11Z

lib/python/agentic_isolation/tests/test_providers.py

+        proc = await asyncio.create_subprocess_exec(
+            "printf", "line1\nline2\nline3\n",
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )
+        lines = [line async for line in BaseProvider._read_stream_lines(proc)]
+        assert lines == ["line1", "line2", "line3"]
+
+    async def test_skips_empty_lines(self) -> None:
+        """Should skip empty lines in output."""
+        import asyncio
+
+        from agentic_isolation.providers.base import BaseProvider
+
+        proc = await asyncio.create_subprocess_exec(
+            "printf", "hello\n\nworld\n",
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )
+        lines = [line async for line in BaseProvider._read_stream_lines(proc)]
+        assert lines == ["hello", "world"]
+
+    async def test_timeout_kills_process(self) -> None:
+        """Should kill the process when timeout is reached."""
+        import asyncio
+
+        from agentic_isolation.providers.base import BaseProvider
+
+        proc = await asyncio.create_subprocess_exec(
+            "sleep", "60",
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+        )


These new async subprocess tests rely on external Unix utilities (printf/sleep/true). If this package or its tests are expected to run on non-GNU environments (e.g., Windows, minimal containers), this can make the suite fail. Consider using sys.executable with "-c" snippets to generate output/sleep in a portable way, or mark these tests with a platform skip.

- Run ruff format on 7 files - Bump observability plugin 0.2.0 → 0.2.1 - Bump sdlc plugin 1.3.0 → 1.3.1

Await proc.wait() after proc.kill() in _terminate_process() so the process is always reaped. Addresses review feedback.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T01:20:21Z

lib/python/agentic_isolation/agentic_isolation/config.py

+    if value is None:
+        if spec.get("required", False):
+            description = spec.get("description", "")
+            raise ValueError(f"Plugin '{plugin_name}' requires env var {var_name}: {description}")


The ValueError message always appends : {description} even when description is empty, resulting in a trailing colon/space (e.g. ... requires env var FOO: ). Consider conditionally including the description only when it’s non-empty to keep the error message clean and consistent.

Suggested change

raise ValueError(f"Plugin '{plugin_name}' requires env var {var_name}: {description}")

if description:

message = f"Plugin '{plugin_name}' requires env var {var_name}: {description}"

else:

message = f"Plugin '{plugin_name}' requires env var {var_name}"

raise ValueError(message)

Copilot · 2026-03-24T01:20:22Z

lib/python/agentic_isolation/agentic_isolation/providers/claude_cli/event_parser.py

+        handlers: dict[str, Any] = {
+            "system": self._handle_system,
+            "assistant": self._handle_assistant,
+            "user": self._handle_user,
+            "result": self._handle_result,
+        }
+
+        handler = handlers.get(event_type)


handlers is rebuilt on every parse_line() call, which adds avoidable allocations in a hot path (this runs once per streamed JSONL line). Consider making the mapping a class attribute (or initializing once in __init__) to keep the refactor while avoiding per-line dict creation.

Suggested change

handlers: dict[str, Any] = {

"system": self._handle_system,

"assistant": self._handle_assistant,

"user": self._handle_user,

"result": self._handle_result,

}

handler = handlers.get(event_type)

if not hasattr(self, "_handlers"):

self._handlers: dict[str, Any] = {

"system": self._handle_system,

"assistant": self._handle_assistant,

"user": self._handle_user,

"result": self._handle_result,

}

handler = self._handlers.get(event_type)

) * refactor: reduce cognitive complexity across all modules (ISS-280) Extract helper functions to bring all 18 source functions below CC≤25 and LOC≤100 thresholds. Add shared _read_stream_lines() to BaseProvider (eliminates duplication between docker/local providers). Add 19 regression tests for extracted helpers. * fix: apply ruff formatting + bump plugin versions - Run ruff format on 7 files - Bump observability plugin 0.2.0 → 0.2.1 - Bump sdlc plugin 1.3.0 → 1.3.1 * fix: reap process after kill to prevent zombies Await proc.wait() after proc.kill() in _terminate_process() so the process is always reaped. Addresses review feedback.

Copilot AI review requested due to automatic review settings March 24, 2026 00:57

Copilot started reviewing on behalf of NeuralEmpowerment March 24, 2026 00:57 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

NeuralEmpowerment added 2 commits March 23, 2026 18:08

fix: apply ruff formatting + bump plugin versions

ce2aa0d

- Run ruff format on 7 files - Bump observability plugin 0.2.0 → 0.2.1 - Bump sdlc plugin 1.3.0 → 1.3.1

fix: reap process after kill to prevent zombies

f9c837a

Await proc.wait() after proc.kill() in _terminate_process() so the process is always reaped. Addresses review feedback.

Copilot AI review requested due to automatic review settings March 24, 2026 01:15

Copilot started reviewing on behalf of NeuralEmpowerment March 24, 2026 01:15 View session

NeuralEmpowerment merged commit 084be02 into main Mar 24, 2026
10 checks passed

Copilot AI reviewed Mar 24, 2026

View reviewed changes

NeuralEmpowerment deleted the refactor/iss-280-reduce-complexity branch March 27, 2026 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: reduce cognitive complexity across all modules (ISS-280)#117

refactor: reduce cognitive complexity across all modules (ISS-280)#117
NeuralEmpowerment merged 3 commits intomainfrom
refactor/iss-280-reduce-complexity

NeuralEmpowerment commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            raise ValueError(f"Plugin '{plugin_name}' requires env var {var_name}: {description}")
+            if description:
+                message = f"Plugin '{plugin_name}' requires env var {var_name}: {description}"
+            else:
+                message = f"Plugin '{plugin_name}' requires env var {var_name}"
+            raise ValueError(message)

Conversation

NeuralEmpowerment commented Mar 24, 2026

Summary

Refactored modules

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants