Repository navigation
feat(source): add ‘source clean’ command - #261
Merged
Merged
Conversation
added 2 commits
April 8, 2026 21:27
Adds `notebooklm source clean` to automatically remove junk sources from a notebook in bulk. Useful after failed deep-research runs, bot-blocked crawls, or bulk imports that left behind error or duplicate sources. Removes sources that match any of: - Status is 'error' or 'unknown' - Title matches gateway/anti-bot patterns (403, 404, Access Denied, Cloudflare 'Just a Moment', CAPTCHA, etc.) - URL is a duplicate of an already-seen source (keeps oldest) Flags: --dry-run Preview what would be deleted without deleting -y/--yes Skip confirmation prompt -n Target a specific notebook Deletions are batched in chunks of 10 with a 0.5s delay to avoid hitting rate limits on large notebooks.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a ChangesNew Source Clean Command
Sequence DiagramsequenceDiagram
actor User
participant CLI as "CLI"
participant Resolver as "resolve_notebook_id"
participant SourceSvc as "sources.list / sources.delete"
participant Selector as "identify_candidates"
participant Normalizer as "normalize_url"
participant Deleter as "chunked_deleter"
User->>CLI: source clean --notebook X [--dry-run] [--yes]
CLI->>Resolver: resolve_notebook_id(X)
Resolver-->>CLI: notebook_id
CLI->>SourceSvc: list_sources(notebook_id)
SourceSvc-->>CLI: sources[]
CLI->>Selector: identify_candidates(sources)
Selector->>Normalizer: normalize_url(src.url)
Normalizer-->>Selector: normalized_url (fragment stripped)
Selector-->>CLI: candidate_ids[] (status/title/duplicate reasons)
alt dry-run
CLI->>User: report candidate count & table
else proceed
CLI->>User: prompt confirmation (unless --yes)
User-->>CLI: confirm
CLI->>Deleter: delete_in_batches(candidate_ids, size=10)
loop per batch
Deleter->>SourceSvc: delete_source(id) (concurrent via asyncio.gather)
SourceSvc-->>Deleter: success / exception
Deleter->>Deleter: sleep(0.5s)
end
Deleter-->>CLI: successes & failures
CLI->>User: final report (errors truncated to first 5)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/notebooklm/cli/source.py`:
- Around line 982-992: The deletion loop currently swallows all errors by
calling asyncio.gather(..., return_exceptions=True) and always prints success;
update the loop around client.sources.delete / asyncio.gather to inspect the
gathered results, count actual successful deletions vs exceptions, and log or
print any failures (include exception messages and the corresponding sid) as
they occur; after the loop, change the console.print call to report the real
number of successful deletions (and optionally failed count) using the counted
successes so users aren’t shown a false “Successfully cleaned” message for
delete_list, chunk_size, delete_tasks, and nb_id_resolved.
- Around line 945-950: The code incorrectly converts the enum s.status to a
string with str(...).lower(), so comparisons never match; replace that
conversion with the helper source_status_to_str(s.status) (handle None if
needed) and use that result to check if status is "error" or "unknown", then add
s.id to to_delete as before (look at variables s, to_delete, and function
source_status_to_str for where to change).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ae7d06c-d332-4bb1-9a3a-f0412794bf36
📒 Files selected for processing (1)
src/notebooklm/cli/source.py
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new source clean command to the CLI, designed to automatically remove duplicate, error, and access-blocked sources from a NotebookLM notebook. The command identifies sources based on their status, title, and normalized URL, and includes options for dry-run and confirmation. A critical issue was identified in the status checking logic: the s.status enum is incorrectly converted to a string, which prevents sources in an error or unknown state from being properly detected and cleaned. The suggested fix is to use the source_status_to_str() helper function.
- Use source_status_to_str() instead of str().lower() so error/unknown status sources are correctly identified and removed - Track per-result success/failure from asyncio.gather so the output reflects what actually happened instead of always reporting success Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/notebooklm/cli/source.py (1)
957-965: URL dedup misses case-only differences and trailing slashes.
urlparsepreserves the scheme/host casing and raw path, soHTTPS://Example.com/aandhttps://example.com/a/will not dedupe againsthttps://example.com/a, even though they point at the same resource. Lowercasing scheme + netloc and stripping a trailing/on the path gives better duplicate coverage without affecting correctness.♻️ Proposed tweak
- parsed = urlparse(url) - normalized = urlunparse((parsed.scheme, parsed.netloc, parsed.path, "", "", "")) + parsed = urlparse(url) + path = parsed.path.rstrip("/") or "/" + normalized = urlunparse( + (parsed.scheme.lower(), parsed.netloc.lower(), path, "", "", "") + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/notebooklm/cli/source.py` around lines 957 - 965, The current URL normalization misses case differences and trailing slashes; update the normalization logic around urlparse/urlunparse so scheme and netloc are lowercased and the path has a single canonical form (strip any trailing slash except preserve root as "/" or empty path as ""), then use that normalized string for deduping. Concretely: take parsed = urlparse(url), set scheme = parsed.scheme.lower(), netloc = parsed.netloc.lower(), compute path_norm = parsed.path.rstrip('/') (and if path_norm == '' and parsed.path == '/' keep '/' or otherwise use '' per your desired canonicalization), then call urlunparse((scheme, netloc, path_norm, "", "", "")) and use that for seen_urls/to_delete comparisons (same variables: normalized, seen_urls, to_delete, s.id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/notebooklm/cli/source.py`:
- Around line 952-955: The current junk-filtering deletes any source whose title
starts with "http://" or "https://", which is too broad; update the logic around
GATEWAY_PATTERNS and the title.startswith(("http://", "https://")) check so it
only drops URL-title sources when the fetch clearly failed—i.e., gate that
branch behind the source status check (use status in ["error","unknown"]) or
remove the URL-prefix heuristic entirely; modify the block that adds s.id to
to_delete (referencing GATEWAY_PATTERNS, the title.startswith check, and the
status variable) so in-progress/ready items are not silently purged.
---
Nitpick comments:
In `@src/notebooklm/cli/source.py`:
- Around line 957-965: The current URL normalization misses case differences and
trailing slashes; update the normalization logic around urlparse/urlunparse so
scheme and netloc are lowercased and the path has a single canonical form (strip
any trailing slash except preserve root as "/" or empty path as ""), then use
that normalized string for deduping. Concretely: take parsed = urlparse(url),
set scheme = parsed.scheme.lower(), netloc = parsed.netloc.lower(), compute
path_norm = parsed.path.rstrip('/') (and if path_norm == '' and parsed.path ==
'/' keep '/' or otherwise use '' per your desired canonicalization), then call
urlunparse((scheme, netloc, path_norm, "", "", "")) and use that for
seen_urls/to_delete comparisons (same variables: normalized, seen_urls,
to_delete, s.id).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2da1f3e1-768a-42a6-ad0b-62fb7e078798
📒 Files selected for processing (1)
src/notebooklm/cli/source.py
Owner
|
This is a destructive command, so it needs more care before we can merge: (1) make |
Polish pass on the new `source clean` command after a 3-way review
(Claude/Gemini/Codex) flagged data-loss risks and a missing test suite.
Safety fixes
- Drop the `title.startswith("http://"...)` heuristic. Source.title is
documented as "may be URL if not yet processed", so the previous code
would silently delete legitimate sources whose metadata had not yet
resolved. The gateway-title regex still catches the obvious
Cloudflare/CAPTCHA/403 blocking pages.
- Remove "unknown" from `_JUNK_STATUSES`. Only explicit `error` is
cleaned; future NotebookLM status codes we don't recognize and
missing-status payloads are now preserved instead of wiped.
- Preserve query string in URL normalization. Stripping it collapsed
distinct YouTube videos (`?v=AAA` vs `?v=BBB`), Google Docs IDs, and
arXiv versions. Only the `#fragment` is stripped now.
- Push undated sources to the end of the sort (was: epoch-0, which made
them the "oldest" anchor during dedup so real dated copies got
deleted).
UX
- Dry-run and confirmation now print a Rich table of id/title/status/
reason instead of just a count, so the user can audit what the
heuristics matched before authorizing a destructive operation.
- Partial-failure output lists up to 5 failing source IDs with their
exception messages so the user can retry or investigate.
Refactor
- Extract pure `_classify_junk_sources()` helper so the classification
logic is unit-testable without spinning up the Click command.
- Add `_print_clean_candidates()` for the table rendering.
Tests
- TestSourceCleanClassify: 15 unit tests covering each branch (error
status, gateway titles, dedup with oldest-first ordering, dedup when
oldest is error, query-string preservation, fragment stripping,
undated-source ordering, no-URL no-dedup, unknown/zero/processing
statuses not flagged).
- TestSourceCleanCommand: 5 end-to-end Click tests covering
already-clean, dry-run, --yes, declined confirmation, and partial
delete failures.
All 88 source tests + 2635 full-suite tests pass; ruff/mypy clean.
Owner
|
Taking over this PR for finalization. Pushed polish commit Polish pass (after a 3-way review by Claude / Gemini / Codex):
Live validation on a real notebook: created a notebook with 6 sources (1 errored + 3 Local checks: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/cli/test_source.py (1)
1800-1826: 💤 Low valueConsider
contextlib.ExitStackfor the patch stack.The hand-rolled
__enter__/__exit__pair leaks state if a laterpatch(...).__enter__()raises: previously-entered patches won't be exited because the current__enter__doesn't wrap subsequent enters in try/except.contextlib.ExitStackis the idiomatic Python primitive for this and gives you safe rollback for free.♻️ Sketch
-import importlib +import importlib +from contextlib import ExitStack ... class _CleanPatch: def __init__(self, sources): self._sources = sources - self._stack: list = [] + self._stack = ExitStack() def __enter__(self): - mock_client_cls_ctx = patch_client_for_module("source") - mock_client_cls = mock_client_cls_ctx.__enter__() - self._stack.append(mock_client_cls_ctx) - - mock_client = create_mock_client() - mock_client.sources.list = AsyncMock(return_value=self._sources) - mock_client.sources.delete = AsyncMock(return_value=None) - mock_client_cls.return_value = mock_client - self.sources = mock_client.sources - - fetch_ctx = patch("notebooklm.auth.fetch_tokens_with_domains", new_callable=AsyncMock) - fetch_mock = fetch_ctx.__enter__() - fetch_mock.return_value = ("csrf", "session") - self._stack.append(fetch_ctx) - return self + self._stack.__enter__() + try: + mock_client_cls = self._stack.enter_context(patch_client_for_module("source")) + mock_client = create_mock_client() + mock_client.sources.list = AsyncMock(return_value=self._sources) + mock_client.sources.delete = AsyncMock(return_value=None) + mock_client_cls.return_value = mock_client + self.sources = mock_client.sources + + fetch_mock = self._stack.enter_context( + patch("notebooklm.auth.fetch_tokens_with_domains", new_callable=AsyncMock) + ) + fetch_mock.return_value = ("csrf", "session") + except BaseException: + self._stack.__exit__(*sys.exc_info()) + raise + return self - def __exit__(self, *exc): - for ctx in reversed(self._stack): - ctx.__exit__(*exc) + def __exit__(self, *exc): + return self._stack.__exit__(*exc)🤖 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/unit/cli/test_source.py` around lines 1800 - 1826, Replace the manual enter/exit stack in class _CleanPatch with contextlib.ExitStack: initialize self._stack = ExitStack() in __init__, replace calls to patch_client_for_module(...).__enter__() and patch(...).__enter__() with self._stack.enter_context(patch_client_for_module("source")) and self._stack.enter_context(patch("notebooklm.auth.fetch_tokens_with_domains", new_callable=AsyncMock)), and use self._stack.close() (or rely on ExitStack.__exit__) in __exit__ so any failure during enter will automatically roll back previously-entered contexts; keep references to mock_client_cls and fetch_mock from the enter_context returns and retain self.sources assignment as before.
🤖 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.
Inline comments:
In `@src/notebooklm/cli/source.py`:
- Around line 58-66: _normalize_url_for_dedup currently preserves original
casing of scheme and netloc so equivalent URLs with different casing (e.g.
HTTPS://Example.com) won't dedupe; update the function to normalize scheme and
host to lowercase before urlunparse (lowercase parsed.scheme and parsed.netloc
or use parsed.hostname with port handling) while still preserving path, params,
and query and stripping fragment, and add a unit test in TestSourceCleanClassify
that supplies mixed-case schemes/hosts (e.g. "HTTPS://Example.com/a" vs
"https://example.com/a") to assert they produce the same normalized key.
---
Nitpick comments:
In `@tests/unit/cli/test_source.py`:
- Around line 1800-1826: Replace the manual enter/exit stack in class
_CleanPatch with contextlib.ExitStack: initialize self._stack = ExitStack() in
__init__, replace calls to patch_client_for_module(...).__enter__() and
patch(...).__enter__() with
self._stack.enter_context(patch_client_for_module("source")) and
self._stack.enter_context(patch("notebooklm.auth.fetch_tokens_with_domains",
new_callable=AsyncMock)), and use self._stack.close() (or rely on
ExitStack.__exit__) in __exit__ so any failure during enter will automatically
roll back previously-entered contexts; keep references to mock_client_cls and
fetch_mock from the enter_context returns and retain self.sources assignment as
before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 028e233f-fab3-4c57-87f1-217c66c574ba
📒 Files selected for processing (2)
src/notebooklm/cli/source.pytests/unit/cli/test_source.py
Follow-up from CodeRabbit review on commit 489d9a3: - `_normalize_url_for_dedup` now lowercases scheme and netloc per RFC 3986 (both are case-insensitive), so `https://Example.COM/a` dedups against `https://example.com/a`. Path/params/query/fragment handling unchanged. - `_CleanPatch` test helper uses `contextlib.ExitStack` so a mid-setup exception in any patch correctly unwinds the patches that already entered, instead of leaking them into the next test. Adds `test_dedup_is_case_insensitive_on_scheme_and_host` to lock in the behaviour.
7 tasks done
4 tasks
teng-lin
pushed a commit
that referenced
this pull request
May 21, 2026
Restructures the [0.5.0] - UNRELEASED section to lead with what users need to act on, drop internal-refactoring prose, and stay accurate against the actual commits since v0.4.1. Structure (Keep a Changelog + upfront Breaking summary): Breaking changes (13 items) → Added (Auth/Chat/CLI/Python API) → Changed → Deprecated → Removed → Fixed → Security Notable additions surfaced by the triple-model review (claude + codex + agy) and verified against gh pr view: - Breaking: sources.add_* kw-only (#756), server_error_max_retries=3 default (#629), max_concurrent_rpcs=16 default (#630), --storage context-isolation script-impact (#467). - Added: NOTEBOOKLM_BASE_URL enterprise (#402), source clean (#261), create --use (#220/#413), Chromium profile selectors (#648), public rpc_call (#646), observability+drain (#643), correlation IDs + categorized logging (#430/#431), upload timeouts (#618), ChatRef enhancements (#686), citation hover save (#675). - Changed: server-assigned conversation_id (#659/#667), cross-event- loop fail-fast (#633). - Security: bundled #746 + #803 + #903 into one consolidated entry. No source-code changes — CHANGELOG.md only.
teng-lin
pushed a commit
that referenced
this pull request
May 21, 2026
Restructures the [0.5.0] - UNRELEASED section to lead with what users need to act on, drop internal-refactoring prose, and stay accurate against the actual commits since v0.4.1. Structure (Keep a Changelog + upfront Breaking summary): Breaking changes (13 items) → Added (Auth/Chat/CLI/Python API) → Changed → Deprecated → Removed → Fixed → Security Notable additions surfaced by the triple-model review (claude + codex + agy) and verified against gh pr view: - Breaking: sources.add_* kw-only (#756), server_error_max_retries=3 default (#629), max_concurrent_rpcs=16 default (#630), --storage context-isolation script-impact (#467). - Added: NOTEBOOKLM_BASE_URL enterprise (#402), source clean (#261), create --use (#220/#413), Chromium profile selectors (#648), public rpc_call (#646), observability+drain (#643), correlation IDs + categorized logging (#430/#431), upload timeouts (#618), ChatRef enhancements (#686), citation hover save (#675). - Changed: server-assigned conversation_id (#659/#667), cross-event- loop fail-fast (#633). - Security: bundled #746 + #803 + #903 into one consolidated entry. No source-code changes — CHANGELOG.md only.
teng-lin
added a commit
that referenced
this pull request
May 21, 2026
) Restructures the [0.5.0] - UNRELEASED section to lead with what users need to act on, drop internal-refactoring prose, and stay accurate against the actual commits since v0.4.1. Structure (Keep a Changelog + upfront Breaking summary): Breaking changes (13 items) → Added (Auth/Chat/CLI/Python API) → Changed → Deprecated → Removed → Fixed → Security Notable additions surfaced by the triple-model review (claude + codex + agy) and verified against gh pr view: - Breaking: sources.add_* kw-only (#756), server_error_max_retries=3 default (#629), max_concurrent_rpcs=16 default (#630), --storage context-isolation script-impact (#467). - Added: NOTEBOOKLM_BASE_URL enterprise (#402), source clean (#261), create --use (#220/#413), Chromium profile selectors (#648), public rpc_call (#646), observability+drain (#643), correlation IDs + categorized logging (#430/#431), upload timeouts (#618), ChatRef enhancements (#686), citation hover save (#675). - Changed: server-assigned conversation_id (#659/#667), cross-event- loop fail-fast (#633). - Security: bundled #746 + #803 + #903 into one consolidated entry. No source-code changes — CHANGELOG.md only. Co-authored-by: Claude <claude@zfs.local>
This was referenced May 21, 2026
zeekay
pushed a commit
to Dream-AI-4444/notebooklm-py
that referenced
this pull request
Sep 9, 2026
Adds `notebooklm source clean` — bulk-remove junk sources (errored, gateway/anti-bot-blocked, duplicates) from a notebook. ## What it removes - Sources with `status=error` - Sources whose title matches a known gateway / anti-bot pattern (Cloudflare "Just a Moment", Access Denied, 403, 404, CAPTCHA, etc.) - Duplicate URLs — fragment stripped, query preserved, scheme/host lowercased per RFC 3986; oldest copy of each URL is kept ## Flags ``` notebooklm source clean # interactive confirmation notebooklm source clean --dry-run # preview as a Rich table without deleting notebooklm source clean -y # skip confirmation notebooklm source clean -n <id> # target specific notebook ``` ## Implementation notes - Dry-run and confirmation print a Rich table of id / title / status / reason so the user can audit before authorizing a destructive operation. - Deletions are chunked (10 per batch with a 0.5s sleep between chunks) using the existing `asyncio.gather` pattern. - Partial failures collect `(id, exception)` tuples and show up to 5 failing source IDs with their messages. - Pure `_classify_junk_sources()` helper is unit-testable without spinning up the Click command — 16 classify tests + 5 end-to-end Click tests covering every branch. Co-authored-by: Flosters <flosters@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After failed deep-research runs, bulk imports, or bot-blocked crawls, notebooks can accumulate junk sources: errored ones, gateway/anti-bot pages (Cloudflare, 403, 404), and duplicates. There is currently no way to clean these up in bulk — users must delete them one by one.
This PR adds
notebooklm source clean, which automatically identifies and removes junk sources in one command.What it removes
statusiserrororunknown403,404,Forbidden,Access Denied,Just a Moment,Attention Required,Security Check,CAPTCHAFlags
Implementation notes
asyncio.gatherpattern consistent with other bulk operations in the codebaseTest plan
--dry-runprints count and exits without deletingstatus=errorare removed"403 Forbidden") are removed"Notebook is already clean"-yskips confirmation promptSummary by CodeRabbit
New Features
notebooklm source cleanCLI to find and optionally remove junk sources (status errors, gateway/security blocks, duplicate URLs). Keeps oldest copy per URL, preserves query strings, strips fragments, and prints a summary table. Supports--dry-run(preview) and--yes(skip confirmation); performs batched deletions with brief pauses and reports successes or top failures.Tests