test(e2e): scenarios 2 + 3 (#334 step 2) - #363
Conversation
#334) Adds tests/e2e/test_source_type_discrimination.py covering the three first-class ingest paths (cli_remember, filesystem/transcript, git) through their real console-script entry points and asserting their discriminating identifiers in ingest_log. Catches the #190 R1 class of regression where an INGEST_SOURCE_* constant is defined but never written by the path that should use it. A new conftest fixture installed_console_script resolves siblings of the installed aelf binary so the test can invoke aelf-commit-ingest across the uv-tool / pipx / venv-pip matrix.
Adds tests/e2e/test_hook_inject_roundtrip.py covering the seam the UserPromptSubmit / PreCompact hooks travel: pre-seeded store + recent transcript -> rebuild_v14 -> stdout context block. Two cases: a locked belief related to a recent turn must land in the block; an empty store + empty transcript must exit cleanly (no-op contract for fresh projects). Catches drift in the transcript reader, retrieval, or block renderer that would silently inject empty memory.
Reviewer's GuideAdds two new end-to-end scenarios to validate ingest source discrimination across CLI/filesystem/git paths and the hook→ingest→rebuild roundtrip, plus a shared fixture for resolving console scripts across installation methods. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 15 minutes and 56 seconds.Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
installed_console_script, consider importing and usingcollections.abc.Callablefor the return type instead of the string-annotated"callable[[str], Sequence[str]]"plus# type: ignore[name-defined], so the fixture is properly typed without suppressing type checking. - The docstring for
test_three_paths_record_distinct_source_typesstill refers tosource_type/belief_corroborations, but the implementation asserts oningest_log.source_kind; updating the wording to match the actual assertion target would make the intent clearer. - In
test_three_paths_record_distinct_source_types, the assertionshort_hash in commit_proc.stdout or commit_proc.returncode == 0is redundant becausecheck=Truealready guaranteesreturncode == 0; you can simplify this check or drop it if only the non-zero exit status is relevant.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `installed_console_script`, consider importing and using `collections.abc.Callable` for the return type instead of the string-annotated `"callable[[str], Sequence[str]]"` plus `# type: ignore[name-defined]`, so the fixture is properly typed without suppressing type checking.
- The docstring for `test_three_paths_record_distinct_source_types` still refers to `source_type`/`belief_corroborations`, but the implementation asserts on `ingest_log.source_kind`; updating the wording to match the actual assertion target would make the intent clearer.
- In `test_three_paths_record_distinct_source_types`, the assertion `short_hash in commit_proc.stdout or commit_proc.returncode == 0` is redundant because `check=True` already guarantees `returncode == 0`; you can simplify this check or drop it if only the non-zero exit status is relevant.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[claim:review:Setr:2026-05-03T03:25:43Z] |
|
[claim:review:Toug:2026-05-03T03:25:47Z] |
|
[release:review:Toug:2026-05-03T03:25:52Z] |
|
[claim:review:Gylf:2026-05-03T03:26:13Z] |
|
[release:review:Gylf:2026-05-03T03:26:18Z] |
|
[release:review:Setr:2026-05-03T03:28:43Z] |
Summary
Step 2 of #334 rollout: adds scenarios #3 and #2 from the ratified spec.
tests/e2e/test_source_type_discrimination.py: exercises the three first-class ingest paths (cli_remember,filesystem/transcript,git) through their real console-script entry points (aelf lock,aelf ingest-transcript,aelf-commit-ingest) and asserts the discriminating identifiers land iningest_log. Catches the [phantom-prereqs T1] Belief corroboration tracking — sibling table + ingest recorder #190 R1 class — a constant defined in module A but never written by path B.tests/e2e/test_hook_inject_roundtrip.py: exercises store → rebuild → stdout context block (the same code path the UserPromptSubmit/PreCompact hooks call). Asserts a locked belief related to a recent transcript turn reaches the rebuild output, and that empty input exits cleanly (no-op contract).conftest.py— addsinstalled_console_scriptfixture for sibling resolution of console scripts (aelf-commit-ingest) across the uv-tool / pipx / venv-pip matrix.After the corroboration table proved unsuitable for a one-shot test (rows only fire on content-hash hits), the source-discrimination test pivoted to
ingest_log.source_kind, which is the v2.0 #205 source-of-truth log written on everyrecord_ingest. The bug class (#190 R1 — constant defined but never written) is caught either way;ingest_logis the durable single-shot signal.Test plan
uv run pytest tests/e2e/ -v— all 6 cases pass locally (3 from step 1 + 3 new)ruff check tests/e2e/— cleansrc/aelfrice/hook_commit_ingest.pye2eworkflow runs on this PR via thee2elabel opt-in (per step 1)Closes part of #334 step 2; spec gate "4 of 6 seed scenarios pass" advances from 1/4 to 3/4.
Summary by Sourcery
Add new end-to-end coverage for ingest source discrimination and hook-driven rebuild roundtrips.
Tests: