Export: latest-day filter, project-scoped CLI, and bulk API hardening#34
Open
leostar0412 wants to merge 3 commits intocppalliance:masterfrom
Open
Export: latest-day filter, project-scoped CLI, and bulk API hardening#34leostar0412 wants to merge 3 commits intocppalliance:masterfrom
leostar0412 wants to merge 3 commits intocppalliance:masterfrom
Conversation
…nd latest-day slice - Updated README to clarify bulk export options, including incremental updates and latest-day slice. - Modified CLI export flags to support `--since incremental` for exporting only new or changed sessions since the last export. - Implemented logic in the export API to handle the new export options, including returning a 422 error when there is nothing to export. - Enhanced session filtering for the latest activity day and improved project matching for exports. - Added unit tests for new export features and validation of state management.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds UTC calendar-day filtering for "last" exports, a concurrency-safe atomic export-state store, an "incremental" export mode, CLI project matching by display name, API/CLI/UI wiring for the modes, tests, and README updates. ChangesIncremental Export & Calendar-Day Sessions with Improved Project Matching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Possibly related reviewers
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 5
🤖 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 `@api/export_api.py`:
- Around line 118-121: The handler currently treats invalid or misspelled since
values as "all" which causes accidental full exports; update the validation
around request.get_json(...) and the since variable so that if body.get("since")
exists but is not one of ("all","last","incremental") the endpoint returns a 400
(or 422) JSON error response instead of falling back to "all". Keep the allowed
set check (since in ("all","last","incremental")), but change the branch to
return a proper JSON error (including a short message and the invalid value) and
only proceed when since is valid or absent; reference the local variable since
and the request.get_json call to locate where to add the error response.
In `@README.md`:
- Line 22: Update the "Bulk export" documentation text to show the full HTTP 422
JSON response structure instead of just `Nothing to export`: replace the short
note with a clear example such as stating the API returns 422 JSON with
{"error": "Nothing to export", "since": <value>} (include both "error" and
"since" fields) so consumers can parse the response correctly; edit the sentence
mentioning Bulk export on the same line to include that exact JSON structure.
- Line 73: Update the README text that describes the `--since last` zip filename
to reflect the actual pattern shown in the example: change the statement that
says `last-MM-DD` to indicate the full pattern
`claude-code-export-last-MM-DD-YYYY-MM-DD.zip`, where the first MM-DD is the
latest calendar day (UTC) and the trailing YYYY-MM-DD is the export timestamp;
edit the descriptive sentence near the `--since last` explanation and any nearby
example text so they consistently show
`claude-code-export-last-MM-DD-YYYY-MM-DD.zip`.
In `@scripts/export.py`:
- Around line 464-465: The CLI still writes export_state.json with a plain
open(..., "w") in _save_state(), risking races with the API path that uses
atomic write+lock; change _save_state() to use the same atomic/locked write used
by the Flask/API code (i.e., acquire the same state lock, write to a temp file,
fsync, then os.replace/rename) and keep _load_state() reads protected by that
lock as well; ensure you validate/serialize the JSON into the temp file before
replacing and surface errors instead of leaving a truncated/invalid
export_state.json.
In `@utils/export_day_filter.py`:
- Around line 61-64: The try/except around parse_session silently swallows parse
errors (parse_session called with sess_info["path"]) which can make d = max(...)
pick the wrong day; change the except to capture the exception as e and record
it (e.g., logger.error or logger.exception) including sess_info["path"] and the
exception message, and then either re-raise the exception or propagate the
choice to the caller via a new parameter (e.g., abort_on_parse_error) so the
caller can decide to abort (raise) or continue (log+continue). Ensure the unique
symbols referenced are parse_session, sess_info["path"], and the downstream
latest-day computation (d = max(...)) so reviewers can locate and verify the
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: e92e036d-4dd1-4c23-b59f-00d04ed6a876
📒 Files selected for processing (9)
README.mdapi/export_api.pyscripts/export.pystatic/js/app.jstests/test_cli_args.pytests/test_export_api_bulk.pytests/test_export_day_filter.pytests/test_export_project_filter.pyutils/export_day_filter.py
…r handling - Introduced a new utility module for managing export state with atomic I/O and locking mechanisms. - Updated the export API to validate request bodies and handle invalid 'since' parameters, returning appropriate error responses. - Enhanced the README to clarify bulk export options and CLI export flag behaviors. - Added unit tests for new error handling scenarios in the export API and improved logging for session parsing failures. - Refactored existing code to utilize the new export state management utilities, ensuring consistency across API and CLI.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
api/export_api.py (1)
79-83:⚠️ Potential issue | 🟠 MajorMalformed JSON still falls back to a full export.
request.get_json(silent=True)returnsNoneon decode failure, and this branch treats that the same as “no body”. A client sending invalid JSON withContent-Type: application/jsoncan still reachsince="all"instead of getting a 400/422.🤖 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 `@api/export_api.py` around lines 79 - 83, The current request.get_json(silent=True) call in export_api.py masks JSON decode errors by returning None and causing malformed JSON to be treated as an empty body; change the JSON parsing to detect decode errors by calling request.get_json(silent=False) (or wrapping request.get_json(...) in a try/except catching werkzeug.exceptions.BadRequest/JSONDecodeError) so that if parsing fails you return a 400/422 JSON error instead of falling back to empty body; update the handling around the body variable and the existing isinstance(body, dict) check so only a truly missing body (None) or an object proceeds, while malformed JSON yields an immediate client error response.
🤖 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 `@scripts/export.py`:
- Around line 91-143: The helper _prefixed_export_option_overrides only scans
for "export" and misses shared flags for other subcommands like "list" and
"stats"; update it to detect the first subcommand token among the command set
(e.g. "export", "list", "stats") instead of only "export" and parse the argv
prefix up to that token so shared flags (--project, --since, --out, etc.) are
recovered for all those subcommands; also rename or keep the function but update
any call sites (the usages around the previous 150-152 region) to use the
generalized helper so project scoping is consistent across list and stats as
well.
In `@utils/export_state_store.py`:
- Around line 11-17: The fallback using threading.Lock (variables
_fallback_locks and _fallback_locks_guard) only serializes threads in one
process; replace it with a cross-process file lock on Windows (use
msvcrt.locking on a per-store lock file) so CLI and Flask processes cannot
interleave R/W; keep the same locking API but have the fallback map lock keys to
file handles/lockfiles and perform msvcrt.locking/unlocking around critical
sections (import msvcrt when fcntl is unavailable and update places that use
_fallback_locks to acquire/release the file lock instead of threading.Lock).
- Around line 60-67: The current loader that opens path, calls json.load(path)
and returns data may return non-dict payloads or a non-dict "sessions" value;
update the loader to validate and sanitize the JSON shape: after loading into
data, ensure isinstance(data, dict) (otherwise return {"sessions": {}} or {}),
ensure the "sessions" key exists and that data["sessions"] is a dict (if missing
or not a dict set data["sessions"] = {}), and leave or validate "lastExportTime"
as-is; return the sanitized dict so callers that expect a mapping (references to
data and data["sessions"]) never receive non-mapping types.
---
Duplicate comments:
In `@api/export_api.py`:
- Around line 79-83: The current request.get_json(silent=True) call in
export_api.py masks JSON decode errors by returning None and causing malformed
JSON to be treated as an empty body; change the JSON parsing to detect decode
errors by calling request.get_json(silent=False) (or wrapping
request.get_json(...) in a try/except catching
werkzeug.exceptions.BadRequest/JSONDecodeError) so that if parsing fails you
return a 400/422 JSON error instead of falling back to empty body; update the
handling around the body variable and the existing isinstance(body, dict) check
so only a truly missing body (None) or an object proceeds, while malformed JSON
yields an immediate client error response.
🪄 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: bfc3bbe0-e5f0-478d-96eb-69525424b1ad
📒 Files selected for processing (7)
README.mdapi/export_api.pyscripts/export.pytests/test_export_api_bulk.pytests/test_export_day_filter.pyutils/export_day_filter.pyutils/export_state_store.py
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_export_day_filter.py
- utils/export_day_filter.py
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
1 participant
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
Improves the export story end-to-end: shared latest calendar-day handling for
--since last, project filtering in the CLI, and more reliable bulk export in the web API (state, empty runs, and incremental behavior aligned with the CLI).What’s in this change
utils/export_day_filter.py) — centralizes “latest activity day” and session overlap logic for exports that target the most recent UTC day.scripts/export.py) — supports--since lastwith the new helper, project scoping, and keeps incremental / full export behavior consistent with the API where it matters.api/export_api.py) —all/last/incrementalmodes; no empty zip when there’s nothing to export (HTTP 422 + JSON); clearer export state payload (last_export_session_countand legacyexport_count); locked + atomic updates toexport_state.jsonto avoid torn reads and lost merges.static/js/app.js) — handles 422 from bulk export with a clear message; shows “sessions in last export” using the new state fields.README.md) — documents new behavior (e.g. empty bulk export response).How to test
Manually: run the app, try Export all / Export new since last with and without new sessions; confirm a no-op export shows a clear error (not a tiny empty zip). From the CLI, exercise
--since lastand--projectagainst a small projects directory.Closes #33
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
--since,--projectmatching, and export filename semantics.Tests