From e9e2deb090d4bb3490f171708f72beebb03d2de3 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Thu, 7 May 2026 10:46:16 +0000 Subject: [PATCH] refactor(agent): replace hardcoded dependency and context file limits with configurable parameters author CleverThis 1776170939 +0000 committer CleverThis 1776170939 +0000 refactor(agent): replace hardcoded dependency and context file limits with configurable parameters Implemented configurable limits for the agent and graph components: - ContextAnalysisAgent now accepts max_dependencies: int = 10 with validation (ValueError if <= 0) - _parse_dependencies uses self.max_dependencies instead of a hard-coded 10 - PlanGenerationGraph now accepts max_context_files: int = 5 with validation (ValueError if <= 0) - _format_context_summary uses self.max_context_files instead of a hard-coded 5 - Updated class docstrings to reflect new parameters - Added Behave feature file at features/agent_configurable_limits.feature with 12 scenarios - Added step definitions at features/steps/agent_configurable_limits_steps.py ISSUES CLOSED: #9050 --- CHANGELOG.md | 10 +- CONTRIBUTORS.md | 1 + features/agent_configurable_limits.feature | 83 ++++++++ .../steps/agent_configurable_limits_steps.py | 191 ++++++++++++++++++ .../agents/graphs/context_analysis.py | 14 +- .../agents/graphs/plan_generation.py | 21 +- 6 files changed, 315 insertions(+), 5 deletions(-) create mode 100644 features/agent_configurable_limits.feature create mode 100644 features/steps/agent_configurable_limits_steps.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 961570993..adaeda325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -855,6 +855,14 @@ uko-oo:Class` triple emission in `PythonAnalyzer._extract_class()` so that reflecting the outcome. Plans with no DoD text skip evaluation and proceed normally. +### Changed + +- **Configurable Agent Limits** (#9246): Replaced hardcoded `deps[:10]` in + ``ContextAnalysisAgent`` and ``contexts[:5]` in ``PlanGenerationGraph`` with + configurable constructor parameters ``max_dependencies`` (default: 10) and + ``max_context_files`` (default: 5). Non-positive values raise ``ValueError``. + All existing call sites remain backward-compatible via default arguments. + ### Added - **ACMS Index Data Model and File Traversal Engine** (#9579): Implements the @@ -1279,4 +1287,4 @@ iteration` and data corruption under concurrent plan execution. All public - **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget` renders permission requests directly in the conversation stream for single-key operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`), - navigate with arrow keys, confirm with `Enter`, or press `v` to open the full + navigate with arrow keys, confirm with `Enter`, or press `v` to open the full \ No newline at end of file diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 08cf0f1fb..e0b305e7b 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -21,6 +21,7 @@ Below are some of the specific details of various contributions. * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements. * HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool. * HAL 9000 has contributed concurrency safety improvements, including thread-safe context tier management (issue #7547) for parallel plan execution. +* HAL 9000 has contributed the configurable agent limits refactor (#9246/#9050): replaced hardcoded ``deps[:10]`` in ``ContextAnalysisAgent`` and ``contexts[:5]`` in ``PlanGenerationGraph`` with validated constructor parameters ``max_dependencies`` (default: 10) and ``max_context_files`` (default: 5), including 12 BDD scenarios covering defaults, custom values, edge cases, and invalid-input error handling. * HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption. * HAL 9000 has contributed the bug-hunt-pool-supervisor non-blocking tracking fix (#7875 / PR #7957): updated step 5 to be best-effort and added rule 9 to prevent the automation-tracking-manager call from blocking the main supervisor loop. * Jeffrey Phillips Freeman has contributed the complete AUTO-BUG-POOL to AUTO-BUG-SUP tracking prefix fix across agent-system-specification.md, automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875). diff --git a/features/agent_configurable_limits.feature b/features/agent_configurable_limits.feature new file mode 100644 index 000000000..78344ebac --- /dev/null +++ b/features/agent_configurable_limits.feature @@ -0,0 +1,83 @@ +Feature: Configurable limits in context analysis and plan generation + As a developer using the agents module + I want to configure the maximum number of dependencies and context files + So that I can tune the agents for different project sizes and resource constraints + + Background: + Given the configurable limits module is importable + + # ContextAnalysisAgent.max_dependencies tests + + @configurable_limits + Scenario: ContextAnalysisAgent uses default max_dependencies of 10 + Given a ContextAnalysisAgent with default settings + When I parse dependencies from a string with 15 items + Then the result should contain exactly 10 dependencies + + @configurable_limits + Scenario: ContextAnalysisAgent respects custom max_dependencies + Given a ContextAnalysisAgent with max_dependencies set to 3 + When I parse dependencies from a string with 15 items + Then the result should contain exactly 3 dependencies + + @configurable_limits + Scenario: ContextAnalysisAgent raises ValueError for non-positive max_dependencies + When I create a ContextAnalysisAgent with max_dependencies set to 0 + Then a ValueError should be raised about max_dependencies + + @configurable_limits + Scenario: ContextAnalysisAgent raises ValueError for negative max_dependencies + When I create a ContextAnalysisAgent with max_dependencies set to -5 + Then a ValueError should be raised about max_dependencies + + @configurable_limits + Scenario: ContextAnalysisAgent max_dependencies of 1 returns single dependency + Given a ContextAnalysisAgent with max_dependencies set to 1 + When I parse dependencies from a string with 15 items + Then the result should contain exactly 1 dependencies + + # PlanGenerationGraph.max_context_files tests + + @configurable_limits + Scenario: PlanGenerationGraph uses default max_context_files of 5 + Given a PlanGenerationGraph with default settings + When I format a context summary with 8 context files + Then the summary should contain exactly 5 file entries + And the summary should mention 3 more files + + @configurable_limits + Scenario: PlanGenerationGraph respects custom max_context_files + Given a PlanGenerationGraph with max_context_files set to 2 + When I format a context summary with 8 context files + Then the summary should contain exactly 2 file entries + And the summary should mention 6 more files + + @configurable_limits + Scenario: PlanGenerationGraph raises ValueError for non-positive max_context_files + When I create a PlanGenerationGraph with max_context_files set to 0 + Then a ValueError should be raised about max_context_files + + @configurable_limits + Scenario: PlanGenerationGraph raises ValueError for negative max_context_files + When I create a PlanGenerationGraph with max_context_files set to -1 + Then a ValueError should be raised about max_context_files + + @configurable_limits + Scenario: PlanGenerationGraph with fewer contexts than max_context_files shows all + Given a PlanGenerationGraph with max_context_files set to 10 + When I format a context summary with 3 context files + Then the summary should contain exactly 3 file entries + And the summary should not mention more files + + @configurable_limits + Scenario: PlanGenerationGraph with exactly max_context_files contexts shows all + Given a PlanGenerationGraph with default settings + When I format a context summary with 5 context files + Then the summary should contain exactly 5 file entries + And the summary should not mention more files + + @configurable_limits + Scenario: PlanGenerationGraph with empty contexts returns no context message + Given a PlanGenerationGraph with default settings + When I format a context summary with 0 context files + Then the summary should be the no-context message diff --git a/features/steps/agent_configurable_limits_steps.py b/features/steps/agent_configurable_limits_steps.py new file mode 100644 index 000000000..a15ea2b5d --- /dev/null +++ b/features/steps/agent_configurable_limits_steps.py @@ -0,0 +1,191 @@ +"""Step definitions for agent_configurable_limits.feature. + +These steps test the configurable ``max_dependencies`` parameter on +``ContextAnalysisAgent`` and the configurable ``max_context_files`` parameter +on ``PlanGenerationGraph``, covering: + +- Default values preserve existing behaviour (10 / 5 respectively) +- Custom values are respected +- Non-positive values raise ``ValueError`` +""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then, when +from langchain_community.llms import FakeListLLM + +from cleveragents.agents.graphs.context_analysis import ContextAnalysisAgent +from cleveragents.agents.graphs.plan_generation import PlanGenerationGraph +from cleveragents.domain.models.core import Context + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_DEFAULT_RESPONSES = [ + "Dependencies: ['os', 'sys', 'pathlib']", + "Relevance: 0.8 - core module", + "Summary: Generated context overview", +] * 30 + + +def _make_llm() -> FakeListLLM: + return FakeListLLM(responses=list(_DEFAULT_RESPONSES)) + + +def _make_dep_string(count: int) -> str: + """Return an LLM-style dependency string with *count* items.""" + items = ", ".join(f"dep_{i}" for i in range(count)) + return f"[{items}]" + + +def _make_contexts(count: int) -> list[Context]: + """Return a list of *count* dummy Context objects.""" + return [ + Context(plan_id=1, path=f"src/file_{i}.py", content=f"content of file {i}") + for i in range(count) + ] + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("the configurable limits module is importable") +def step_module_importable(context: Any) -> None: + assert ContextAnalysisAgent is not None + assert PlanGenerationGraph is not None + + +# --------------------------------------------------------------------------- +# ContextAnalysisAgent — default max_dependencies +# --------------------------------------------------------------------------- + + +@given("a ContextAnalysisAgent with default settings") +def step_create_agent_default(context: Any) -> None: + context.agent = ContextAnalysisAgent(llm=_make_llm()) + + +@given("a ContextAnalysisAgent with max_dependencies set to {limit:d}") +def step_create_agent_custom_max_deps(context: Any, limit: int) -> None: + context.agent = ContextAnalysisAgent(llm=_make_llm(), max_dependencies=limit) + + +@when("I parse dependencies from a string with {count:d} items") +def step_parse_deps_with_count(context: Any, count: int) -> None: + dep_string = _make_dep_string(count) + context.parsed_deps = context.agent._parse_dependencies(dep_string) + + +@then("the result should contain exactly {expected:d} dependencies") +def step_check_dep_count(context: Any, expected: int) -> None: + actual = len(context.parsed_deps) + assert actual == expected, ( + f"Expected {expected} dependencies, got {actual}: {context.parsed_deps!r}" + ) + + +# --------------------------------------------------------------------------- +# ContextAnalysisAgent — ValueError for invalid max_dependencies +# --------------------------------------------------------------------------- + + +@when("I create a ContextAnalysisAgent with max_dependencies set to {limit:d}") +def step_create_agent_invalid_max_deps(context: Any, limit: int) -> None: + context.raised_error = None + try: + ContextAnalysisAgent(llm=_make_llm(), max_dependencies=limit) + except ValueError as exc: + context.raised_error = exc + + +@then("a ValueError should be raised about max_dependencies") +def step_check_value_error_max_deps(context: Any) -> None: + assert context.raised_error is not None, ( + "Expected ValueError but no error was raised" + ) + assert "max_dependencies" in str(context.raised_error), ( + f"Expected 'max_dependencies' in error message, got: {context.raised_error!r}" + ) + + +# --------------------------------------------------------------------------- +# PlanGenerationGraph — default max_context_files +# --------------------------------------------------------------------------- + + +@given("a PlanGenerationGraph with default settings") +def step_create_graph_default(context: Any) -> None: + context.graph = PlanGenerationGraph(llm=_make_llm()) + + +@given("a PlanGenerationGraph with max_context_files set to {limit:d}") +def step_create_graph_custom_max_ctx(context: Any, limit: int) -> None: + context.graph = PlanGenerationGraph(llm=_make_llm(), max_context_files=limit) + + +@when("I format a context summary with {count:d} context files") +def step_format_context_summary(context: Any, count: int) -> None: + contexts = _make_contexts(count) + context.summary = context.graph._format_context_summary(contexts) + context.context_count = count + + +@then("the summary should contain exactly {expected:d} file entries") +def step_check_file_entries(context: Any, expected: int) -> None: + # Each file entry starts with "File: " + file_count = context.summary.count("File: ") + assert file_count == expected, ( + f"Expected {expected} 'File:' entries in summary, got {file_count}.\n" + f"Summary:\n{context.summary!r}" + ) + + +@then("the summary should mention {extra:d} more files") +def step_check_more_files_mention(context: Any, extra: int) -> None: + assert f"... and {extra} more files" in context.summary, ( + f"Expected '... and {extra} more files' in summary.\n" + f"Summary:\n{context.summary!r}" + ) + + +@then("the summary should not mention more files") +def step_check_no_more_files(context: Any) -> None: + assert "more files" not in context.summary, ( + f"Expected no 'more files' mention in summary.\nSummary:\n{context.summary!r}" + ) + + +@then("the summary should be the no-context message") +def step_check_no_context_message(context: Any) -> None: + assert context.summary == "No context files provided", ( + f"Expected 'No context files provided', got: {context.summary!r}" + ) + + +# --------------------------------------------------------------------------- +# PlanGenerationGraph — ValueError for invalid max_context_files +# --------------------------------------------------------------------------- + + +@when("I create a PlanGenerationGraph with max_context_files set to {limit:d}") +def step_create_graph_invalid_max_ctx(context: Any, limit: int) -> None: + context.raised_error = None + try: + PlanGenerationGraph(llm=_make_llm(), max_context_files=limit) + except ValueError as exc: + context.raised_error = exc + + +@then("a ValueError should be raised about max_context_files") +def step_check_value_error_max_ctx(context: Any) -> None: + assert context.raised_error is not None, ( + "Expected ValueError but no error was raised" + ) + assert "max_context_files" in str(context.raised_error), ( + f"Expected 'max_context_files' in error message, got: {context.raised_error!r}" + ) diff --git a/src/cleveragents/agents/graphs/context_analysis.py b/src/cleveragents/agents/graphs/context_analysis.py index aea8289b0..f976d3edc 100644 --- a/src/cleveragents/agents/graphs/context_analysis.py +++ b/src/cleveragents/agents/graphs/context_analysis.py @@ -97,6 +97,7 @@ class ContextAnalysisAgent: chunk_size: int = 2000, chunk_overlap: int = 200, retry_attempts: int = 3, + max_dependencies: int = 10, ): """Initialize the context analysis agent. @@ -105,10 +106,21 @@ class ContextAnalysisAgent: chunk_size: Maximum size of each chunk in characters chunk_overlap: Overlap between chunks in characters retry_attempts: Number of times to retry transient LLM failures + max_dependencies: Maximum number of dependencies returned by + ``_parse_dependencies``. Must be a positive integer. + Defaults to ``10``. + + Raises: + ValueError: If ``max_dependencies`` is not a positive integer. """ + if max_dependencies <= 0: + raise ValueError( + f"max_dependencies must be a positive integer, got {max_dependencies!r}" + ) self.chunk_size = chunk_size self.chunk_overlap = chunk_overlap self.retry_attempts = max(1, retry_attempts) + self.max_dependencies = max_dependencies # Initialize LLM - an LLM must be provided explicitly if llm is None: @@ -291,7 +303,7 @@ class ContextAnalysisAgent: .split(",") ) deps.extend(part.strip() for part in parts if part.strip()) - return deps[:10] + return deps[: self.max_dependencies] def _chunk_documents(self, state: ContextAnalysisState) -> dict[str, Any]: """Chunk large documents for processing.""" diff --git a/src/cleveragents/agents/graphs/plan_generation.py b/src/cleveragents/agents/graphs/plan_generation.py index 398f64fbd..71b903a64 100644 --- a/src/cleveragents/agents/graphs/plan_generation.py +++ b/src/cleveragents/agents/graphs/plan_generation.py @@ -167,6 +167,7 @@ class PlanGenerationGraph: max_retries: int = 3, context_llm: BaseLanguageModel | None = None, checkpoint_limit: int = 2, + max_context_files: int = 5, ): """Initialize the plan generation graph. @@ -175,7 +176,19 @@ class PlanGenerationGraph: max_retries: Maximum number of retry attempts context_llm: Optional language model dedicated to context analysis checkpoint_limit: Maximum checkpoints to retain per thread + max_context_files: Maximum number of context files included in + ``_format_context_summary``. Must be a positive integer. + Defaults to ``5``. + + Raises: + ValueError: If ``max_context_files`` is not a positive integer. """ + if max_context_files <= 0: + raise ValueError( + "max_context_files must be a positive integer, " + f"got {max_context_files!r}" + ) + self.max_context_files = max_context_files self.max_retries = max(1, max_retries) # Initialize LLMs - an LLM must be provided explicitly @@ -619,12 +632,14 @@ class PlanGenerationGraph: return "No context files provided" summary_parts: list[str] = [] - for ctx in contexts[:5]: # Limit to first 5 files + for ctx in contexts[: self.max_context_files]: content_preview = ctx.content[:300] if ctx.content else "" summary_parts.append(f"File: {ctx.path}\nPreview: {content_preview}...\n") - if len(contexts) > 5: - summary_parts.append(f"... and {len(contexts) - 5} more files") + if len(contexts) > self.max_context_files: + summary_parts.append( + f"... and {len(contexts) - self.max_context_files} more files" + ) return "\n".join(summary_parts) -- 2.52.0