5 Commits

Author	SHA1	Message	Date
brent.edwards	eb46f0ff54	fix(plan): add tier hydration and improve architecture review output (#10938) ... ## Summary This PR fixes issue #10878 where architecture reviews were truncated because the regex pattern for parsing file output would stop at the first ``` encountered in the Markdown report. ## Changes - Change file delimiters from ``` to >>>>>>>/<<<<<<< to avoid Markdown conflicts - Add tier hydration before strategize phase in plan_executor.py - Increase max_tokens to 16384 in llm_actors.py for longer outputs - Increase context_max_tokens_hot from 16000 to 32000 in settings.py - Fix get_hot_view → get_hot_fragments in strategy_actor.py and plan_executor.py - Add opencode to skip directories in context_tier_hydrator.py - Change sandbox output location to plan-output/ directory in plan.py - Add get_context_summary stub method to acms_service.py ## Testing Run architecture review action and verify the output report is complete with all sections. Reviewed-on: #10938	2026-05-19 12:43:34 +00:00
hamza.khyari	a740d9c15a	fix: address PR review findings - narrow exception scope and fix test mocks ... Address CoreRasurae's review comments: 1. Narrow exception scope in _build_decisions (line 846) to catch only json.JSONDecodeError and ValidationError instead of bare Exception. This prevents swallowing unintended errors like memory issues. 2. Update feature file section header and scenario titles from _run_execute_with_stub to _run_execute_with_actor for consistency. 3. Add spec to MagicMock in failing execute scenario (line 138) to match the pattern used in succeeding scenario (line 119). Note: Master feature entry point - Behave auto-discovers all .feature files, no explicit entry point needed. Tests run successfully. ISSUES CLOSED: #10874	2026-04-30 14:56:33 +00:00
CoreRasurae	d3cb534caf	feat(plan): implement LLM-powered strategy actor ... Implement StrategyActor class for the plan strategize phase that uses an LLM to produce hierarchical execution strategies with dependencies, resource requirements, estimated complexity, and risk scores. Key components: - StrategyActor: Core actor with LLM prompt construction, response parsing (JSON and numbered-list fallback), and graceful degradation to StrategizeStubActor when no LLM provider is configured - StrategyAction/StrategyTree: Pydantic models for the hierarchical action tree with dependency links - validate_no_cycles(): Kahns algorithm (deque-based) for dependency graph cycle detection, raising PlanError on circular dependencies - build_strategy_prompt(): Context-aware prompt construction using definition_of_done, resources, project context, and ACMS analysis with XML-delimited user content sections for prompt injection hardening - parse_strategy_response(): Robust LLM output parsing with JSON extraction and numbered-list fallback - resolve_strategy_actor(): Integration point for the existing actor.default.strategy config key (CLEVERAGENTS_DEFAULT_STRATEGY_ACTOR) - Decision conversion producing strategy_choice Decision objects - build_decisions() preserves tree hierarchy via parent_id mapping, populates downstream_decision_ids from dependency edges, and validates plan_id Structural tree hierarchy (B2 review fix): - _build_tree infers parent_id from the dependency graph: each actions first resolved dependency becomes its structural parent. Actions with no dependencies fall back to the root. This produces hierarchical trees for agents plan tree rendering per spec Plan Decision Tree. Downstream decision tracking (B3 review fix): - build_decisions populates downstream_decision_ids from the strategy trees dependency edges using a pre-generated decision_id map so influence relationships between decisions are recorded per the spec Decision Record Structure. Post code-review hardening (PR #1175): - Broadened exception handling in execute() and ACMS retrieval to catch all LLM provider errors (openai, httpx, anthropic, etc.) with graceful fallback to stub mode (H1, H2) - Added warning log for unresolvable dependency references so dropped edges are visible in structured logs (H3) - Added XML-delimited user content sections and explicit data-only instructions in system prompt for prompt injection hardening (H4) - Switched prompt truncation to word-boundary-safe _truncate_at_word() for all prompt input sections (M1) - Fixed _parse_actor_name to preserve user-specified provider or model when only one segment is empty, instead of discarding both (M2) - Annotated _build_invariant_records as placeholder pending the Invariant Reconciliation Actor implementation (M5) - Documented resources/project_context params as future-wired through PlanExecutor.run_strategize() (M8) - Added docstring noting supersession relationship with LLMStrategizeActor in llm_actors.py (M9) - Added __all__ export definition (L2) - Improved validate_no_cycles docstring edge direction semantics (L7) - Cap JSON parse retry loop at _MAX_JSON_PARSE_RETRIES (10) Post second code-review hardening (PR #1175, review cycle 2): - Fixed _truncate_at_word docstring: documented max_chars >= 3 precondition for the result-length guarantee (R-H1) - Added warning log in build_decisions for unresolvable parent_id references, matching the existing _build_tree warning for unresolvable dependency references (R-H2) - Fixed _parse_actor_name to handle whitespace-only input by adding actor_name.strip() check alongside the emptiness check (R-M1) - Tightened ACMS scenario assertions from non-empty to expected count of 5 decisions (R-L3) - Added timeout=60s on_timeout=kill to all Robot test cases for consistency with project patterns (R-M5) Post third code-review hardening (PR #1175, review cycle 3): - Added _sanitize_xml_content() to escape XML special characters (<, >, &) in user content before embedding into XML-delimited prompt sections, preventing prompt injection via forged closing tags (spec Prompt Injection Mitigation) (CR3-M1) - Upgraded _try_parse_json() to multi-anchor retry: collects all [{ positions left-to-right and tries each as a candidate start, fixing false-start anchoring when LLM preamble contains [{ fragments before the real JSON array (CR3-M2) - Added _truncate_at_word() guard for max_chars < 3: returns a hard slice instead of word-boundary truncation when the ellipsis would exceed the limit (CR3-L2) - Changed _build_tree collision fallback key from -(idx+1) to -(1_000_000+idx) to eliminate theoretical collision with LLM-produced negative step numbers (CR3-L3) - Added forward-looking API docstring note to build_decisions() documenting that it is not yet wired into PlanExecutor and will be integrated once Decision persistence lands (CR3-M3) Post fourth code-review hardening (PR #1175, review cycle 4): - Fixed _try_parse_json per-anchor retry counter: reset retries=0 at the start of each anchor iteration so false-start [{ anchors in LLM preamble text no longer exhaust the retry budget for the correct anchor (CR4-B1) - Added known-limitations docstring to module header documenting missing decision types (resource_selection, subplan_spawn, invariant_enforced) as future work (CR4-D1) - Rewrote XML injection assertion in test to use regex extraction instead of fragile chained .split() calls that could IndexError on structural changes (CR4-T5) Post fifth code-review hardening (PR #1175, review cycle 5): - Added warning log in build_decisions for empty-string parent_id (distinct from None) so the silent fallback to root is visible in structured logs for debuggability (CR5-B1) - Added plan_id propagation assertion to build_decisions test scenarios verifying decision.plan_id matches the input (CR5-T1) - Added sequence_number monotonicity assertion verifying decision sequence_numbers are zero-indexed and monotonically increasing (CR5-T2) - Added _truncate_at_word boundary test for max_chars=3 (exactly ellipsis length) verifying correct "..." output (CR5-T3) - Tightened false-start anchor test from permissive len>=1 to specific description match "Sole real action" (CR5-T4) - Added word-boundary truncation test using space-separated input to exercise the rfind(" ") path under oversized DoD (CR5-T5) Post sixth code-review hardening (PR #1175, review cycle 6): - Added _MAX_INVARIANTS cap (100) for invariant list truncation in prompt to prevent token limit overflows, consistent with other prompt section caps (CR6-M4) - Added negative max_chars guard in _truncate_at_word returning empty string instead of slicing from end (CR6-M5) - Added global JSON parse attempt cap _MAX_GLOBAL_JSON_ATTEMPTS (50) across all anchors in _try_parse_json (CR6-L3) - Moved re import to module level in strategy_parsing.py per CONTRIBUTING import guidelines (CR6-L4) - Extracted _DEFAULT_DESCRIPTION constant to eliminate duplication between _default_action() and _build_tree() (CR6-L5) Post seventh code-review hardening (PR #1175, review cycle 7): - Decoupled _execute_stub from StrategizeStubActor._parse_steps private method by delegating to parse_strategy_response, removing cross-class private method dependency (CR7-M1) - Added ULID format validation on plan_id in execute() and build_decisions() for spec-consistent argument validation per §Plan glossary and CONTRIBUTING §Argument Validation (CR7-M2) - Constrained StrategyAction.estimated_complexity to Literal["low", "medium", "high"] at Pydantic model level per CONTRIBUTING §Type Safety (CR7-M5) - Documented XML-tag prompt boundary deviation from spec [USER_CONTENT_START]/[USER_CONTENT_END] markers with rationale for the more structured approach (CR7-M6) - Added _build_tree empty-input guard comment documenting orphaned root_id semantics (CR7-L1) - Added _truncate_at_word > 0 intent comment explaining why position-0 space is intentionally excluded (CR7-L2) - Added build_decisions context_snapshot future-work comment referencing spec §Decision Record Structure (CR7-L5) - Used enumerate() in _build_tree first loop for idiomatic Python (CR7-L7) - Fixed false-start anchor test (CR5-T4) broken by CR6-L3 global cap: reduced preamble fragments from 15 to 3 so total attempts stay within _MAX_GLOBAL_JSON_ATTEMPTS (CR7-T1) - Fixed test plan_ids containing non-Crockford-Base32 characters (L→K) to pass ULID format validation (CR7-T2) Tests: - 105 Behave BDD scenarios in features/strategy_actor_llm.feature adding: global JSON attempt cap exhaustion (CR7-L3), orphaned dependency edge silent drop (CR7-L4), non-ULID plan_id rejection in execute() and build_decisions() (CR7-M2) - 101 Behave BDD scenarios in features/strategy_actor_llm.feature including new scenarios for _truncate_at_word edge cases (L3), create_llm argument verification (L4), non-numeric step field fallback (L5), updated assertions for XML-delimited prompts and _parse_actor_name partial-segment preservation (M2), lifecycle exception fallback (R1), PydanticValidationError re-raise verification (R2), self-loop cycle detection (R3), whitespace-only actor name (R4), XML tag injection sanitisation (CR3-M1), preamble bracket fragment parsing (CR3-M2), _truncate_at_word sub-3 limit (CR3-L2), resolve_strategy_actor with both llm config and registry (CR3-L5), build_decisions unresolvable parent_id fallback (CR3-L7), XML injection in resources/project_context/acms_context fields (CR4-S1), ampersand escaping (CR4-S1d), false-start anchor retry budget (CR4-T3), non-sequential step edge specificity (CR4-T4), plan_id propagation (CR5-T1), sequence_number monotonicity (CR5-T2), max_chars=3 boundary (CR5-T3), false-start anchor specificity (CR5-T4), word-boundary truncation (CR5-T5), invariant prompt constraints (CR6-M2), invariant XML sanitisation (CR6-M3), invariant truncation cap (CR6-M4), negative max_chars (CR6-M5), and no-space truncation (CR6-L8) - 7 Robot Framework integration tests in robot/strategy_actor.robot - Mock LLM provider in features/mocks/mock_strategy_llm.py All nox stages pass: lint, typecheck, unit_tests (13789 scenarios), integration_tests (1863 passed). integration_tests (1863 passed, 2 pre-existing TDD failures unrelated to this change). ISSUES CLOSED: #828	2026-04-14 19:26:49 +00:00
freemo	c9abb45adf	test(coverage): add Behave BDD scenarios for 9 under-covered modules ... Added 246 new BDD scenarios across 9 feature files to improve unit test coverage for modules that were either entirely untested or had significant coverage gaps: - lock_service_coverage.feature (27 scenarios): validation branches, TTL boundaries, re-entrant acquisition, rollback on exceptions - plan_apply_service_coverage.feature (54 scenarios): operation labels, diff rendering (plain/rich/json), artifact building, validation gate, changeset resolution and cleanup - plan_executor_coverage.feature (51 scenarios): step parsing, execute actor integration, strategize/execute guards, stub retry/recovery, decision tree construction - skill_cli_coverage_r3.feature (22 scenarios): tools refresh, list/show JSON fallback, capability summary errors, remove confirmation - changeset_repository_coverage.feature (39 scenarios): entry/tool repos validation, database error wrapping, domain conversion, SQLite store CRUD operations - repositories_coverage.feature (20 scenarios): get_by_name/namespace errors, list_available filters, delete with ActionInUseError, plan update with invariants/processing_state/error_details - sandbox_copy_on_write_coverage.feature (12 scenarios): create OSError wrapping, get_path state transitions, commit edge cases, rollback errors, cleanup with missing paths - bridge_coverage.feature (8 scenarios): __del__ suppression, async task cancellation, execute_graph message type handling, stream config, state checkpointer - plan_cli_coverage.feature (13 scenarios): legacy apply/list/cd paths, use-action with estimation/invariant actors, lifecycle-apply guards, status errors, error recovery display All 246 scenarios (1105 steps) pass. Step definitions use unique prefixes to prevent ambiguous step conflicts with existing tests. ISSUES CLOSED: #467	2026-02-27 13:47:42 -05:00
freemo	55aee7cf22	fix(test): commit after each add_skill to prevent session GC rollback, and improved coverage. ... The step_register_skills_table step called add_skill in a loop but only committed once at the end. Because SkillRepository.create() obtains a new session per call and only flushes (never commits), the intermediate sessions could be garbage-collected before the final commit, rolling back their transactions on the shared SQLite :memory: connection. Moving _commit_pending inside the loop ensures each skill is durably committed before the next session is created. ISSUES CLOSED: #418	2026-02-24 12:19:04 -05:00