test(research): VCR integration coverage for import_sources_with_verification (real recording)

[teng-lin]

Gap

ResearchAPI.import_sources_with_verification (src/notebooklm/_research.py) — the timeout-tolerant deep-research import path (issue #315) — had unit + e2e coverage but no integration/VCR test locking the actual wire. The raw import_sources RPC has a VCR backfill (research_import_sources_direct.yaml), but the verification wrapper that snapshots the baseline and tolerates IMPORT_RESEARCH timeouts did not.

What this adds

A REAL fast-research recording (tests/cassettes/research_import_verification.yaml, 0.48 MB) of the verification wrapper's happy path, captured live against the API on a self-cleaning scratch notebook:

1. CREATE_NOTEBOOK + 3× ADD_SOURCE (substantive public Wikipedia paragraphs — no PII)
2. START_FAST_RESEARCH → POLL_RESEARCH to completed (via wait_for_completion, which pins the POLL-reported task id)
3. GET_NOTEBOOK (the pre-import baseline sources.list snapshot) + IMPORT_RESEARCH (the import under test)
4. DELETE_NOTEBOOK cleanup (runs in record AND replay)

A fast research is sufficient because the IMPORT_RESEARCH wire is identical fast-vs-deep; this keeps the recording cheap (~30 s) while the deep-only report-markdown-source import path stays unit-tested.

Replay assertions (record_mode=none, <30 s via fast_sleep)

• import_sources_with_verification(...) returns a non-empty list[dict] of {"id": ..., "title": ...} imported entries.
• The cassette's request rpcids include both RPCMethod.GET_NOTEBOOK.value (baseline snapshot) and RPCMethod.IMPORT_RESEARCH.value (the import), asserted by RPCMethod constant — rotation-proof through rpc/types.py, not string literals.
• A 5 MB size-cap guard mirrors the deep-poll recording.

Timeout / verify / retry branch stays unit-tested

The timeout-driven verify-and-retry branch cannot be exercised through VCR — VCR replays the recorded IMPORT_RESEARCH response verbatim and cannot synthesize the client-side RPCTimeoutError that triggers the probe/retry logic. That branch is covered exhaustively by tests/unit/test_research_import_with_verification.py (baseline-snapshot failure, partial-commit retry filtering, report-entry dropping, duplicate suppression, cancellation propagation, …). This VCR test deliberately scopes to the no-timeout happy path so replay stays deterministic and fast.

Verification

• Live recording reached completed + imported sources; scratch notebook deleted (no orphans).
• tests/_guardrails/test_cassettes_clean.py, test_cassette_shapes.py, test_no_pinned_cassette_values.py all pass; manual grep confirms no live SID/HSID/SAPISIDHASH/Bearer/account-email shapes (cookies + body keys scrubbed).
• uv run pytest tests/integration/test_research_import_verification_vcr.py -q → 3 passed in <1 s.
• uv run mypy src/notebooklm --ignore-missing-imports clean; uv run pre-commit run --all-files passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added integration tests covering the research import-with-verification happy path, replaying recorded interactions to validate baseline snapshot + import behavior.
  • Added checks ensuring the recorded cassette contains required RPC interactions and stays under a size cap.
  • Updated test allowlists/fixtures to include the new integration checks.

[teng-lin]

@claude review

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds a new integration test module that uses VCR cassette replay to verify the import_sources_with_verification happy path. The test creates a notebook with text sources, runs web research to completion, executes the import operation, and validates the returned entries. Supporting helpers detect record mode, extract cassette RPC calls, and accelerate async polling during replay.

Changes

Import Sources Verification VCR Testing

Layer / File(s)	Summary
Configuration and cassette helpers tests/integration/test_research_import_verification_vcr.py, tests/_fixtures/integration_allow_no_vcr_files.txt, tests/_fixtures/integration_vcr_allow_no_vcr_nodeids.txt	Module documentation establishes cassette file location, record-mode constant, and client timeout overrides. Helper functions detect record mode via environment, build client kwargs only when recording, and parse cassette YAML to extract all RPC IDs for rotation-proof assertions. Fixture allowlist and nodeid files are updated to include the new test.
Async acceleration fixture tests/integration/test_research_import_verification_vcr.py	fast_sleep fixture monkey-patches asyncio.sleep to a no-op during replay (preserving real sleep during recording) to accelerate the completion polling loop.
Happy-path integration test tests/integration/test_research_import_verification_vcr.py	Primary VCR test creates a scratch notebook, seeds it with three text sources, runs fast web research until completion, calls import_sources_with_verification with the discovered sources, asserts non-empty list of entries with id and title fields, and cleans up via notebook deletion in a finally block.
Cassette validation and size checks tests/integration/test_research_import_verification_vcr.py	Standalone tests verify cassette file existence, assert recorded RPC calls include both baseline GET_NOTEBOOK and IMPORT_RESEARCH, and enforce a sub-5MB size limit with guidance for trimming excess YAML content.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• teng-lin/notebooklm-py#1264: Both PRs strengthen VCR-backed integration coverage around the IMPORT_RESEARCH RPC call—this PR via cassette assertions for import_sources_with_verification, and the linked PR via explicit coverage-gap tests for research.import_sources.

Poem

🐰 Through cassette tapes we dance and play,
Recording flows for tests to replay!
From sources found to imports true,
VCR verifies what fast researches do.
A rabbit grins: "Tests pass — hop on through!"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main changeset: adding VCR integration test coverage for the import_sources_with_verification method with a real recording.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/research-import-verification-vcr

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @teng-lin's task in 5m 4s —— View job

---

Code Review: PR #1568

• Gather context and understand the PR
• Read the integration test file
• Read the cassette file (spot-check structure)
• Review for correctness, coverage, best practices
• Post review findings

---

Summary

The test design, documentation, and happy-path coverage are all high quality and follow the established patterns from test_research_deep_poll_vcr.py. There is one actionable bug that will cause a CI failure, and a couple of minor observations.

---

Bug: Missing taxonomy allowlist entry (will fail CI)

tests/integration/test_research_import_verification_vcr.py introduces test_cassette_under_size_cap decorated with @pytest.mark.allow_no_vcr. Because the module also has pytestmark = [pytest.mark.vcr, skip_no_cassettes], pytest assigns both vcr and allow_no_vcr to this test.

The guardrail at tests/_guardrails/test_integration_allow_no_vcr_allowlist.py calls scripts/test_taxonomy_inventory.py --check, which enforces that every integration file containing an allow_no_vcr-marked test must appear in tests/_fixtures/integration_allow_no_vcr_files.txt. The new file is not listed there, so the check will fail.

Compare: test_research_deep_poll_vcr.py has an identical test_cassette_under_size_cap function and IS in that allowlist file.

Fix needed: add this line to tests/_fixtures/integration_allow_no_vcr_files.txt (keeping alphabetical order):

tests/integration/test_research_import_verification_vcr.py # existing vcr/allow_no_vcr overlap; normalize per test-suite taxonomy cleanup

And add this line to tests/_fixtures/integration_vcr_allow_no_vcr_nodeids.txt (the audit file for vcr AND allow_no_vcr overlaps — not CI-enforced by --check, but currently stale):

tests/integration/test_research_import_verification_vcr.py::test_cassette_under_size_cap # existing vcr/allow_no_vcr overlap; normalize per test-suite taxonomy cleanup

Fix this →

---

Minor observations

_cassette_request_rpcids parameter is vestigial here. The function accepts cassette_name: str — mirroring the version in test_rpc_gap_backfill_vcr.py where it's called with multiple different cassette names. In this file it's only ever called with CASSETTE_NAME. The implementation also uses CASSETTE_PATH.with_name(cassette_name) rather than the simpler CASSETTE_DIR / cassette_name pattern. Neither is wrong, just slightly over-engineered for a single-cassette module. Nit, not a blocker.

test_cassette_records_baseline_snapshot_and_import double-checks CASSETTE_PATH.exists() even though the module-level skip_no_cassettes marker already guards it. Belt-and-suspenders is harmless here.

---

What's done well

• Rotation-proof assertions: using RPCMethod.GET_NOTEBOOK.value and RPCMethod.IMPORT_RESEARCH.value instead of string literals is exactly right for this codebase.
• fast_sleep fixture: correctly preserves live API cadence in record mode and eliminates sleep overhead in replay — identical to the deep-poll module's approach.
• Cassette scrubbing: the preview shows all cookies and auth tokens properly SCRUBBED; no PII.
• try/finally cleanup: scratch notebook is deleted in both record and replay, keeping the cassette self-consistent.
• Record-only client config: _recording_client_kwargs() correctly applies PoolTimeout-resilient settings only in record mode, leaving replay unaffected.
• Scope justification: the docstring clearly explains why fast research is sufficient and why the timeout/retry branch belongs in unit tests — this makes the test boundary obvious to future contributors.
• Size: ~0.48 MB, well under the 5 MB cap.

---

Overall this is a clean and well-designed addition; the taxonomy allowlist fix is the only required change before merge.

[gemini-code-assist]

Code Review

This pull request introduces a new integration test file, test_research_import_verification_vcr.py, which implements VCR replay tests for the import_sources_with_verification happy path. The feedback suggests reusing the existing _is_vcr_record_mode helper using dynamic path-based loading to maintain a single source of truth and prevent namespace clashes in the non-package tests/ directory.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[teng-lin]

Thanks for the review. Addressing the findings:

• Taxonomy allowlist (required) — fixed. Added tests/integration/test_research_import_verification_vcr.py to integration_allow_no_vcr_files.txt (the CI-enforced --check baseline) in 88311e1, and recorded the ::test_cassette_under_size_cap overlap nodeid in integration_vcr_allow_no_vcr_nodeids.txt in 2ed63e6. I deliberately did not regenerate integration_allow_no_vcr_nodeids.txt via --write-report — that file is already stale on main from unrelated test renames (test_artifacts_integration.py etc.) and --check-generated is not a CI gate, so sweeping it in would add ~80 lines of unrelated churn. Kept the change minimal.
• _cassette_request_rpcids parameter is vestigial — keeping the cassette_name parameter for verbatim parity with the test_rpc_gap_backfill_vcr.py original (new code copies the nearest example); it's a harmless single-call nit.
• Double CASSETTE_PATH.exists() check — intentional belt-and-suspenders; the explicit assert gives a clearer "re-record" message than the skip marker.

Also addressed gemini-code-assist's DRY point: _is_record_mode() now delegates to tests.vcr_config._is_vcr_record_mode (2ed63e6).

[coderabbitai]

🧹 Nitpick comments (1)

tests/_fixtures/integration_allow_no_vcr_files.txt (1)

31-31: ⚡ Quick win

Update the comment to accurately reflect the test's nature.

The comment on line 31 describes the entry as an "existing mock-only integration exception," but per the PR objectives, this is a new VCR-backed integration test that intentionally carries both @pytest.mark.allow_no_vcr and VCR cassettes. The comment was apparently copied from the existing entries (lines 1–30) without customization, creating confusion about why a VCR-named file (_vcr suffix) is allowlisted for "no VCR."

Correct the comment to reflect the test's actual status and rationale.

📝 Proposed comment update

-tests/integration/test_research_import_verification_vcr.py # existing mock-only integration exception; migrate per test-suite taxonomy cleanup
+tests/integration/test_research_import_verification_vcr.py # NEW: VCR-backed test with dual `@pytest.mark.allow_no_vcr` + cassette marking (see integration_vcr_allow_no_vcr_nodeids.txt)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/_fixtures/integration_allow_no_vcr_files.txt` at line 31, Update the
comment that currently calls
tests/integration/test_research_import_verification_vcr.py an "existing
mock-only integration exception" to instead state that this is a new VCR-backed
integration test which intentionally includes `@pytest.mark.allow_no_vcr` while
still using VCR cassettes (i.e., an allowlist entry for a VCR-backed test);
locate the entry referencing
tests/integration/test_research_import_verification_vcr.py and replace the
misleading description with a concise line explaining it is a new VCR-backed
test and why it is allowlisted.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/_fixtures/integration_allow_no_vcr_files.txt`:
- Line 31: Update the comment that currently calls
tests/integration/test_research_import_verification_vcr.py an "existing
mock-only integration exception" to instead state that this is a new VCR-backed
integration test which intentionally includes `@pytest.mark.allow_no_vcr` while
still using VCR cassettes (i.e., an allowlist entry for a VCR-backed test);
locate the entry referencing
tests/integration/test_research_import_verification_vcr.py and replace the
misleading description with a concise line explaining it is a new VCR-backed
test and why it is allowlisted.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e425abfe-fb48-4376-9820-bde7817ef003

📥 Commits

Reviewing files that changed from the base of the PR and between 3c9430b and 2ed63e6.

📒 Files selected for processing (4)

• tests/_fixtures/integration_allow_no_vcr_files.txt
• tests/_fixtures/integration_vcr_allow_no_vcr_nodeids.txt
• tests/cassettes/research_import_verification.yaml
• tests/integration/test_research_import_verification_vcr.py

✅ Files skipped from review due to trivial changes (1)

• tests/_fixtures/integration_vcr_allow_no_vcr_nodeids.txt

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/integration/test_research_import_verification_vcr.py