feat: add sync all feature with collection flags (#69) - #71
Conversation
Add `tweethoarder sync` command that syncs all collections by default. Users can specify which collections to sync using inclusion flags: - `sync` - syncs likes, bookmarks, tweets, reposts, replies (not feed) - `sync --likes` - syncs only likes - `sync --likes --bookmarks` - syncs likes and bookmarks - `sync --feed` - syncs feed (excluded from default) Also adds common options: --count, --with-threads, --full Breaking changes: - Removed `sync posts` subcommand (use `sync --tweets --reposts`) - Removed `sync threads` subcommand (use `sync --threads` flag) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Pass with_threads flag from callback to sync_all_async - Add progress parameter to sync_all_async and pass to individual sync functions - Wrap callback in create_sync_progress() for progress bar display - Output "Sync complete." message after sync finishes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe changes add a unified "sync all" CLI entrypoint that routes flags to a new async coordinator which sequentially runs per-collection syncs (likes, bookmarks, tweets, reposts, replies, optional feed). The posts and threads subcommands were removed, docs updated, and tests adjusted/added for the new flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "tweethoarder CLI"
participant Coordinator as "sync_all_async"
participant PerType as "sync_*_async"
participant DB as "Database / Storage"
participant Progress as "Progress"
User->>CLI: run `tweethoarder sync [--likes|--bookmarks|...] [--count N] [--full]`
CLI->>Coordinator: call with include_* flags, count, with_threads, full
Coordinator->>Progress: create/start progress context
loop for each enabled collection
Coordinator->>PerType: call sync_<type>_async(count, with_threads, full, progress)
PerType->>DB: read/update storage (sync results)
PerType->>Progress: update progress
end
Coordinator->>Progress: finalize/close
Coordinator->>CLI: return completion
CLI->>User: print completion message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-01-16T23:14:31.941ZApplied to files:
🧬 Code graph analysis (1)src/tweethoarder/cli/sync.py (1)
✏️ Tip: You can disable this entire section by setting Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/tweethoarder/cli/sync.py`:
- Around line 183-184: The CLI accepts --feed and --threads but never uses them;
update the callback so these flags actually control sync behavior: modify
sync_all_async to accept an include_feed: bool parameter (and an optional
expand_threads/batch_threads: bool parameter) and pass the feed and threads
variables into it from the CLI handler, or if you prefer keeping feed logic
separate call sync_feed_async when feed is True; ensure the CLI callback calls
sync_all_async(..., include_feed=feed, expand_threads=threads) or calls
sync_feed_async() when feed is set and uses the threads flag to enable batch
thread expansion inside sync_all_async or the thread-expansion routine.
- Around line 185-187: The CLI options count and full are never forwarded to the
sync implementation; update the sync_all_async function signature
(sync_all_async(..., count: int | float = float("inf"), full: bool = False)) and
all calls to it so the CLI-provided count and full are passed through, and then
propagate these parameters into where collections are synced (e.g., calls to
sync_collection_async and any per-source sync functions like
sync_twitter_collection_async) replacing hardcoded count=float("inf") and
always-false full usage; ensure sync_collection_async and downstream sync
functions accept and use the count and full arguments.
- Line 199: The sync command is writing to a different DB file than the
subcommands; update the db path used in sync_callback so it matches the other
commands (use get_data_dir() and the same filename "tweethoarder.db"). Locate
the db_path assignment in sync_callback (db_path = get_config_dir() /
"tweets.db") and replace it to build the path with get_data_dir() /
"tweethoarder.db" so all commands (likes(), bookmarks(), etc.) use the same
database file.
🧹 Nitpick comments (4)
src/tweethoarder/cli/sync.py (1)
216-246: Consider returning aggregated results and adding error isolation.The function has
-> Nonereturn type but individual sync functions return{"synced_count": N}. Consider:
- Returning aggregated counts so callers can report what was synced
- Wrapping each sync call in try/except so one failure doesn't prevent syncing other collections
♻️ Example improvement
async def sync_all_async(...) -> dict[str, int]: """Sync all collection types.""" results: dict[str, int] = {} if include_likes: try: r = await sync_likes_async(...) results["likes"] = r["synced_count"] except Exception as e: typer.echo(f"Warning: likes sync failed: {e}") results["likes"] = 0 # ... similar for other collections return resultstests/cli/test_sync_all.py (3)
14-21: Test assertion could be more precise.The assertion
"Missing command" not in clean_output or result.exit_code == 0is permissive and could pass even when the command fails for unexpected reasons. Consider a stricter check once the implementation is stable.♻️ Suggested improvement
def test_sync_without_subcommand_is_handled() -> None: """Running 'sync' without a subcommand should work (not show help).""" - # For now, just verify it doesn't error with "Missing command" result = runner.invoke(app, ["sync"]) - # Strip ANSI escape codes clean_output = re.sub(r"\x1b\[[0-9;]*m", "", result.output) - # Should not show "Missing command" error - we'll implement the callback - assert "Missing command" not in clean_output or result.exit_code == 0 + # Should complete without "Missing command" error + assert "Missing command" not in clean_output
233-241: Consider verifying call arguments intest_sync_callback_calls_sync_all_async.The test only verifies
assert_called_once()but doesn't check that default parameters were passed correctly. This is partially covered bytest_sync_callback_with_no_flags_syncs_all_except_feedbut verifying here too would strengthen the test.
63-67: Add tests to verify--countand--fullare passed tosync_all_async.While
test_sync_accepts_full_optionverifies the flag is accepted, there's no test verifying the value is passed through tosync_all_async. Given the implementation currently ignores these parameters, adding such tests would help catch the regression.💡 Example test
def test_sync_callback_passes_count_to_sync_all_async() -> None: """The sync callback should pass count to sync_all_async.""" from unittest.mock import AsyncMock, patch with patch("tweethoarder.cli.sync.sync_all_async", new_callable=AsyncMock) as mock: runner.invoke(app, ["sync", "--likes", "--count", "50"]) mock.assert_called_once() call_kwargs = mock.call_args[1] assert call_kwargs.get("count") == 50 def test_sync_callback_passes_full_to_sync_all_async() -> None: """The sync callback should pass full flag to sync_all_async.""" from unittest.mock import AsyncMock, patch with patch("tweethoarder.cli.sync.sync_all_async", new_callable=AsyncMock) as mock: runner.invoke(app, ["sync", "--likes", "--full"]) mock.assert_called_once() call_kwargs = mock.call_args[1] assert call_kwargs.get("full") is True
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.claude/settings.local.jsonSPEC.mdsrc/tweethoarder/cli/sync.pytests/cli/test_sync.pytests/cli/test_sync_all.pytests/cli/test_sync_replies.py
💤 Files with no reviewable changes (1)
- tests/cli/test_sync.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Port Twitter API patterns from bird reference implementation: headers must include x-twitter-auth-type: OAuth2Session, x-twitter-active-user: yes, x-twitter-client-language: en; feature flags need ~40 entries; query IDs should have fallback lists for resilience
Type hints required for all code
Use Google style for docstrings
**/*.py: Type hints required for all code
Use Google style for docstrings
Files:
tests/cli/test_sync_replies.pytests/cli/test_sync_all.pysrc/tweethoarder/cli/sync.py
**/*test*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*test*.py: Import shared logic from production code in tests, never duplicate test logic
Create test data factories that generate test data with sensible defaults
Use business-focused test names that describe business value, not technical details
Files:
tests/cli/test_sync_replies.pytests/cli/test_sync_all.py
**/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/test_*.py: Import shared logic from production code in tests, never duplicate in tests
Create test data factory functions that generate test data with sensible defaults
Test names should describe business value, not technical details
Test edge cases and errors in addition to happy paths
New features require tests; bug fixes require regression tests
Files:
tests/cli/test_sync_replies.pytests/cli/test_sync_all.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: tfriedel/tweethoarder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:14:31.941Z
Learning: Applies to **/*.py : Port Twitter API patterns from bird reference implementation: headers must include x-twitter-auth-type: OAuth2Session, x-twitter-active-user: yes, x-twitter-client-language: en; feature flags need ~40 entries; query IDs should have fallback lists for resilience
📚 Learning: 2026-01-16T23:14:31.941Z
Learnt from: CR
Repo: tfriedel/tweethoarder PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:14:31.941Z
Learning: When working on Twitter API issues, consult the ~/projects/bird directory TypeScript reference implementation for headers (x-twitter-auth-type, x-twitter-active-user, user-agent, origin, referer), feature flag builders, and query ID patterns
Applied to files:
.claude/settings.local.json
🧬 Code graph analysis (2)
tests/cli/test_sync_all.py (2)
src/tweethoarder/cli/sync.py (1)
sync_all_async(216-246)src/tweethoarder/storage/database.py (1)
init_database(129-142)
src/tweethoarder/cli/sync.py (1)
src/tweethoarder/config.py (1)
get_config_dir(9-12)
🔇 Additional comments (5)
.claude/settings.local.json (1)
24-25: LGTM!The new
gh issue listpermission is a reasonable addition for development workflows.SPEC.md (2)
219-227: LGTM - documentation structure is clear.The documentation clearly explains the unified sync flow and distinguishes it from individual subcommands. Note: ensure the
--countand--fullflags work as documented once the implementation issues flagged insync.pyare resolved.
275-277: Documentation is correct but implementation doesn't match.The
--countand--fullflags are correctly documented here, but as noted in the sync.py review, these parameters are not forwarded to the sync functions. Once the implementation is fixed, this documentation will be accurate.tests/cli/test_sync_replies.py (1)
159-169: LGTM!Test correctly validates that the
postscommand was removed, with a clear docstring indicating the replacement approach (--tweets --reposts). The assertion handles both possible failure modes appropriately.tests/cli/test_sync_all.py (1)
1-11: LGTM - well-structured test file with comprehensive coverage.Good use of
CliRunnerfor CLI testing and clear module-level setup. Test names describe business behavior rather than technical details.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Code reviewFound 4 issues:
tweethoarder/src/tweethoarder/cli/sync.py Lines 198 to 200 in 781099a
tweethoarder/src/tweethoarder/cli/sync.py Lines 201 to 212 in 781099a
tweethoarder/src/tweethoarder/cli/sync.py Lines 227 to 246 in 781099a
tweethoarder/src/tweethoarder/cli/sync.py Lines 216 to 226 in 781099a 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Fix database path to use get_data_dir()/tweethoarder.db (not tweets.db) - Pass full parameter through to individual sync functions - Pass count parameter through to individual sync functions - Add include_feed parameter and call sync_feed_async when enabled - Remove unused --threads flag (use --with-threads instead) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
tweethoarder synccommand (no subcommand) that syncs all collections by default--likes,--bookmarks,--tweets,--reposts,--replies,--feed--feedis excluded from default sync (must be explicitly requested)--with-threadsand--fulloptions supportsync postsandsync threadssubcommands (use flags instead)Test plan
tests/cli/test_sync_all.pypasstweethoarder sync --helpshows all flagstweethoarder sync --likessyncs only likes with progress bartweethoarder syncsyncs all collections except feedCloses #69
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.