hurui200320
cbf8bcc993
CI / lint (push) Successful in 15s
CI / build (push) Successful in 26s
CI / quality (push) Successful in 35s
CI / typecheck (push) Successful in 40s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 43s
CI / unit_tests (push) Successful in 3m5s
CI / integration_tests (push) Successful in 3m35s
CI / docker (push) Successful in 1m3s
CI / e2e_tests (push) Successful in 5m28s
CI / coverage (push) Successful in 8m5s
CI / benchmark-publish (push) Successful in 20m11s
## Summary Add `robot/e2e/m5_acceptance.robot` with **21 zero-mock E2E test cases** (in addition to existing M5 test suite) covering all M5 (v3.4.0) acceptance criteria: 1. **Context Assembly** — add/list/show/clear files in the context pipeline 2. **Context Scaling** — 10,000+ file project setup with simulate plumbing *(structural)* 3. **Context Policy Configuration** — per-view include/exclude paths, file-size limits 4. **Budget Enforcement** — max_file_size / max_total_size constraint storage *(structural)* 5. **Context Analysis** — ACMS pipeline inspect (tier schema) and simulate (JSON schema) *(structural)* 6. **Plan Execution** — real LLM calls via `openai/gpt-4o-mini` (`plan use` + `plan resume`) ### Structural vs. Behavioural Scope Tests in sections 1b–4 that use `project context simulate` or `inspect` are **structural / plumbing validations** — they verify CLI execution, JSON serialization, and stored configuration but do **not** exercise actual ACMS indexing or budget enforcement because the `ContextTierService` is an in-memory singleton that starts empty per CLI process. Each affected test has a `[Documentation]` note explaining this limitation. Behavioural ACMS validation is deferred until the full indexing pipeline is wired. ### Production Bug Fixes | Fix | File | Description | |-----|------|-------------| | `session.flush()` → `session.commit()` | `project_context.py` | Policy changes silently lost on `session.close()` | | `contextlib.suppress` rollback wrapper | `project_context.py` | Prevents rollback failure from masking original commit exception | | Add `session_factory` DI provider | `container.py` | `project context` commands hit `AttributeError` | | `providers.Factory` → `providers.Singleton` | `container.py` | Avoid creating duplicate engines per call | | Add Gemini API key pattern | `redaction.py` | `AIzaSy...` keys now redacted in logs | ### Review Feedback Addressed (Tenth Pass — @CoreRasurae Review #2410) | # | Severity | Finding | Fix | |---|----------|---------|-----| | P3-1 | Medium | "Clear Context" test tautological — never asserts files were present before clearing | Added `Should Contain ${list_before.stdout} config.py` precondition check after `context-load` and before `clear` | | P3-2 | Medium | Policy/budget verification uses substring matching (`Should Contain 262144`) | Replaced with `Extract JSON From Stdout` + `$rv.get('max_file_size') == 262144` parsed JSON assertions using `resolved_view` dict access | | P3-3 | Medium | Plan resume doesn't verify `phase` value, only existence | Added `Should Not Be Equal As Strings ${phase} queued` assertion to verify plan transitioned from queued | | P3-4 | Medium | Plan JSON extraction inconsistency (`rindex` vs `Extract JSON From Stdout`) | Replaced fragile `rindex`-based extraction with `Extract JSON From Stdout` keyword for consistency | | P3-6 | Medium | Context show summary weak content assertions | Added `Should Not Contain` guards against traceback/error output to reject false positives | | P3-8 | Medium | `_SafeSession` singleton may accumulate dirty state after rollback | Changed `_SafeSession.close()` from pure no-op to `real.rollback()` to reset session state between calls | | P3-14 | Medium | No test for `_save_policy_json` rollback path | Added BDD scenario "Save policy rollback re-raises after commit failure" with monkey-patched commit | | P3-15 | Medium | No test for `_save_policy_json` on nonexistent project | Added BDD scenario "Save policy on nonexistent project row updates zero rows" verifying silent 0-row behavior | | P4-1 | Low | Plan resume TRY/EXCEPT swallows assertion details | Moved field assertions outside TRY block; TRY only guards JSON extraction | | P4-2 | Low | `Safe Parse Json Field` logs stale error context | Fixed to track and report both Strategy 1 and Strategy 2 error contexts separately | | P4-4 | Low | SQLite WAL/SHM files not cleaned in regression test | Added cleanup loop for `-wal` and `-shm` suffixes alongside `.db` file | ### Deferred Items (Out of Scope) | ID | Severity | Reason | |----|----------|--------| | P2-1 | High | `execution_environment` silently dropped on subsequent `context set` — pre-existing production code bug in `_write_policy()`, not introduced by this PR | | P2-2 | High | Unhandled `ValidationError` on corrupt policy blob — pre-existing `_read_policy()` code, not changed by this PR | | P2-3 | High | Silent no-op UPDATE when `ns_projects` row missing — pre-existing `_save_policy_json` logic; this PR only changed error handling | | P3-5 | Medium | Structural tests cannot detect regressions — already honestly documented in every affected test's `[Documentation]` block | | P3-7 | Medium | View inheritance/override behavior not tested — nice-to-have, not in ticket acceptance criteria | | P3-9 | Medium | `context_set` double-writes when `execution_environment` set — pre-existing production logic | | P3-10 | Medium | `budget_tokens=0` silently replaced by default (falsy `or`) — pre-existing production code bug | | P3-11 | Medium | `context set` replaces entire view instead of merging — pre-existing design choice | | P3-12 | Medium | GEMINI_API_KEY propagated but potentially unused — security-first: propagating for redaction testing | | P3-13 | Medium | `reset_container()` doesn't dispose Singleton resources — pre-existing container lifecycle issue | | M5 | Medium | `_build_session_factory` engine never disposed — production code architecture, out of scope for testing ticket | | M6 | Medium | Missing `check_same_thread`/`isolation_level` — production code architecture, out of scope for testing ticket | | L1 | Low | `plan resume` not in spec CLI synopsis — informational | | L2 | Low | Context summary assertions depend on exact CLI wording — acceptable stability risk | | L3 | Low | Gemini regex minimum length slightly loose — acceptable security-first trade-off | | L4 | Low | Missing Google OAuth2 credential patterns — out of scope for this PR | | P4-3 | Low | `Run CLI` keyword duplicated — different purpose (uses `${WS}` as default cwd), not a true duplicate | | P4-5–P4-9 | Low | Various additional E2E coverage gaps — nice-to-have, not in ticket acceptance criteria | ### Quality Gates | Gate | Result | |------|--------| | lint | PASS | | typecheck | PASS (0 errors) | | unit_tests | **393/393** features, 11,210 scenarios | | integration_tests | **1,576/1,576** | | e2e_tests | **37/37** (21 M5 + 12 M6 + 2 smoke + 2 M1) | | coverage_report | **97%** (threshold: 97%) | ### Files Changed | File | Change | |------|--------| | `robot/e2e/m5_acceptance.robot` | **NEW** — 21 E2E test cases with honest structural documentation, parsed JSON assertions, prerequisite skip guards on all sections, safe assertion messages | | `robot/e2e/common_e2e.resource` | `on_timeout=kill` + return code checks + safe key evaluation via `os.environ.get` + fixed stale error logging in `Safe Parse Json Field` | | `robot/e2e/m1_acceptance.robot` | `on_timeout=kill` on git log | | `robot/e2e/m2_acceptance.robot` | `on_timeout=kill` + return code checks + safe assertion messages (no stderr embedding) | | `src/cleveragents/application/container.py` | Add `_build_session_factory` + `session_factory` Singleton | | `src/cleveragents/cli/commands/project_context.py` | `flush()` → `commit()` + `contextlib.suppress` rollback | | `src/cleveragents/shared/redaction.py` | Add Gemini API key pattern | | `noxfile.py` | Propagate `GEMINI_API_KEY` in e2e_tests | | `CHANGELOG.md` | 4 entries for #745 | | `features/application_container_coverage_boost.feature` | Updated title + 3 scenarios | | `features/steps/application_container_coverage_boost_steps.py` | Step defs for `_build_session_factory` | | `features/consolidated_security.feature` | 2 Gemini API key redaction scenarios | | `features/project_context_cli_coverage_boost.feature` | `flush()→commit()` regression test + rollback path + nonexistent project tests | | `features/steps/project_context_cli_coverage_boost_steps.py` | Separate engines for regression test + `try/finally` cleanup + `_SafeSession.close()` state reset + rollback/nonexistent test steps + WAL/SHM cleanup | Closes #745 ISSUES CLOSED: #745 Reviewed-on: #811 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
165 lines
9.1 KiB
Gherkin
165 lines
9.1 KiB
Gherkin
Feature: Project context CLI coverage boost
|
|
Additional Behave scenarios targeting uncovered lines in
|
|
src/cleveragents/cli/commands/project_context.py.
|
|
|
|
Background:
|
|
Given a coverage-boost in-memory database is initialized
|
|
And a project "local/cov-app" exists for coverage boost
|
|
|
|
# --- _load_policy_json returns None (line 106) ---
|
|
# --- _read_policy returns default when raw is None (line 145) ---
|
|
# --- _read_acms_config returns defaults when raw is None (line 164) ---
|
|
Scenario: Reading policy from a fresh project returns defaults
|
|
When I read the stored policy for "local/cov-app"
|
|
Then the policy should be a fresh default policy
|
|
And the ACMS config should equal the defaults
|
|
|
|
# --- _write_policy preserves existing ACMS config (lines 199-201) ---
|
|
Scenario: Writing policy without acms_config preserves existing ACMS config
|
|
Given I seed ACMS config for "local/cov-app" with hot_max_tokens 9999
|
|
When I write a policy for "local/cov-app" without explicit ACMS config
|
|
Then the stored project-context ACMS config hot_max_tokens should be 9999
|
|
|
|
# --- clear default view resets to empty ContextView (line 574) ---
|
|
Scenario: Clearing the default view resets it
|
|
When I run coverage-boost context set on "local/cov-app" with view "default" and include-resource "keep-me"
|
|
And I run coverage-boost context set on "local/cov-app" with view "default" and clear flag
|
|
Then the coverage-boost command should succeed
|
|
And the stored coverage-boost default view include_resources should be empty
|
|
|
|
# --- default_depth as non-integer string (lines 609-610) ---
|
|
Scenario: Setting default_depth to a non-integer string keeps it as string
|
|
When I run coverage-boost context set on "local/cov-app" with default_depth "deep"
|
|
Then the coverage-boost command should succeed
|
|
And the stored ACMS config default_depth should be "deep"
|
|
|
|
# --- execution_environment valid (lines 628-648) ---
|
|
Scenario: Setting a valid execution environment persists it
|
|
When I run coverage-boost context set on "local/cov-app" with execution_environment "host"
|
|
Then the coverage-boost command should succeed
|
|
And the stored execution_environment should be "host"
|
|
|
|
# --- execution_environment invalid (lines 632-638) ---
|
|
Scenario: Setting an invalid execution environment fails
|
|
When I run coverage-boost context set on "local/cov-app" with execution_environment "spaceship"
|
|
Then the coverage-boost command should have exit code 1
|
|
And the coverage-boost output should contain "Invalid execution environment"
|
|
|
|
# --- non-rich output format for context_set (line 663) ---
|
|
Scenario: Context set with JSON output format
|
|
When I run coverage-boost context set on "local/cov-app" with view "default" include-resource "res1" and format "json"
|
|
Then the coverage-boost command should succeed
|
|
And the coverage-boost output should be valid JSON
|
|
|
|
# --- invalid view in context_show (lines 704-707) ---
|
|
Scenario: Context show with invalid view fails
|
|
When I run coverage-boost context show on "local/cov-app" with view "bogus"
|
|
Then the coverage-boost command should have exit code 1
|
|
And the coverage-boost output should contain "Invalid view"
|
|
|
|
# --- strategy_filter in context_inspect (lines 874-878) ---
|
|
Scenario: Context inspect with strategy filter narrows fragments
|
|
Given the tier service has fragments for "local/cov-app" with strategy metadata
|
|
When I run coverage-boost context inspect on "local/cov-app" with strategy filter "tier_a"
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- focus filter in context_inspect (lines 882-885) ---
|
|
Scenario: Context inspect with focus filter narrows fragments
|
|
Given the tier service has fragments for "local/cov-app" with strategy metadata
|
|
When I run coverage-boost context inspect on "local/cov-app" with focus "res:file-alpha"
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- breadth/depth override in context_inspect (lines 897-902) ---
|
|
Scenario: Context inspect with breadth and depth overrides
|
|
When I run coverage-boost context inspect on "local/cov-app" with breadth 5 and depth "detailed"
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- inspect with view set appends phase to title (line 937) ---
|
|
Scenario: Context inspect with view set
|
|
When I run coverage-boost context inspect on "local/cov-app" with view "execute"
|
|
Then the coverage-boost command should succeed
|
|
And the coverage-boost output should contain "execute"
|
|
|
|
# --- temporal scope "recent" and "all" in simulate (lines 301, 303) ---
|
|
Scenario: Simulate with temporal scope recent
|
|
Given I seed ACMS config for "local/cov-app" with temporal_scope "recent"
|
|
When I run coverage-boost context simulate on "local/cov-app" with defaults
|
|
Then the coverage-boost command should succeed
|
|
|
|
Scenario: Simulate with temporal scope all
|
|
Given I seed ACMS config for "local/cov-app" with temporal_scope "all"
|
|
When I run coverage-boost context simulate on "local/cov-app" with defaults
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- budget exceeded breaks loop (line 335), long content truncation (line 339),
|
|
# access_count > 0 relevance (line 344) ---
|
|
Scenario: Simulate truncates fragments exceeding budget and handles long content
|
|
Given the tier service has large fragments for "local/cov-app" exceeding budget
|
|
When I run coverage-boost context simulate on "local/cov-app" with budget 100
|
|
Then the coverage-boost command should succeed
|
|
And the simulation should have fewer fragments than stored
|
|
|
|
# --- simulate with fragments renders rich table (lines 1085-1101) ---
|
|
Scenario: Simulate with fragments shows rich table output
|
|
Given the tier service has fragments for "local/cov-app" with strategy metadata
|
|
When I run coverage-boost context simulate on "local/cov-app" in rich format
|
|
Then the coverage-boost command should succeed
|
|
And the coverage-boost output should contain "Assembled Fragments"
|
|
|
|
# --- invalid temporal_scope in context_set (lines 614-619) ---
|
|
Scenario: Setting an invalid temporal scope fails
|
|
When I run coverage-boost context set on "local/cov-app" with temporal_scope "never"
|
|
Then the coverage-boost command should have exit code 1
|
|
And the coverage-boost output should contain "Invalid temporal scope"
|
|
|
|
# --- context inspect with invalid view (lines 844-848) ---
|
|
Scenario: Context inspect with invalid view fails
|
|
When I run coverage-boost context inspect on "local/cov-app" with view "bogus"
|
|
Then the coverage-boost command should have exit code 1
|
|
And the coverage-boost output should contain "Invalid view"
|
|
|
|
# --- context simulate with invalid view (lines 1032-1036) ---
|
|
Scenario: Context simulate with invalid view fails
|
|
When I run coverage-boost context simulate on "local/cov-app" with view "bogus"
|
|
Then the coverage-boost command should have exit code 1
|
|
And the coverage-boost output should contain "Invalid view"
|
|
|
|
# --- simulate with focus_uris filtering (lines 322-328) ---
|
|
Scenario: Simulate with focus URIs filters fragments
|
|
Given the tier service has fragments for "local/cov-app" with strategy metadata
|
|
When I run coverage-boost context simulate on "local/cov-app" with focus "res:file-alpha"
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- simulate with strategy_hints (line 293, 361) ---
|
|
Scenario: Simulate with strategy hints
|
|
When I run coverage-boost context simulate on "local/cov-app" with strategy hints "bfs,dfs"
|
|
Then the coverage-boost command should succeed
|
|
|
|
# --- _save_policy_json commit() regression test (line 133) ---
|
|
# Exercises _save_policy_json then reads back via a *real* separate
|
|
# session (no _SafeSession) to prove data persists across session
|
|
# boundaries. If commit() were reverted to flush(), this test fails.
|
|
Scenario: Policy saved with commit persists across separate sessions
|
|
When I save a policy for "local/cov-app" via a real session factory and read it back via a separate session
|
|
Then the policy read from the separate session should contain the saved data
|
|
|
|
# --- context_set ACMS overrides for remaining fields (lines 589-622) ---
|
|
Scenario: Context set with all ACMS pipeline overrides
|
|
When I run coverage-boost context set on "local/cov-app" with all ACMS overrides
|
|
Then the coverage-boost command should succeed
|
|
And the stored ACMS config should reflect all overrides
|
|
|
|
# --- _save_policy_json rollback path (contextlib.suppress + re-raise) ---
|
|
# Verifies that when commit() fails, the exception is re-raised and the
|
|
# session is rolled back, releasing DB locks.
|
|
Scenario: Save policy rollback re-raises after commit failure
|
|
When I attempt to save a policy with a commit that will fail
|
|
Then the original commit exception should be re-raised
|
|
And the session should not be in a dirty state after the failure
|
|
|
|
# --- _save_policy_json on nonexistent project (0 rows affected) ---
|
|
Scenario: Save policy on nonexistent project row updates zero rows
|
|
When I save a policy for a nonexistent project "local/no-such-project"
|
|
Then the save should complete without error
|
|
And the policy should not be retrievable for "local/no-such-project"
|