fix(cl2k): heal each asset in the Drive folder that receives its type - #466
Merged
Conversation
poster_self_heal paired every locally-saved row with a single Drive id — the first upload claiming "poster". Now that art types route to their own folders, a generated logo/background/squareart was renamed against the POSTER Drive, where the file does not exist. rclone moveto exits non-zero, apply_proposal raises before the local rename, and the row never heals — silently, on every run. Resolve the twin from the Drive claiming the row's own image_type. A type no Drive claims still falls back to the poster Drive, then to the first Drive at all, preserving the uploads-off behaviour. Extracted as drive_twins() so the mapping is testable; the new tests fail against the previous single-id behaviour.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe poster self-heal module now maps image types to configured Drive folders. Local posters use the matching folder, with poster-folder and first-configured-Drive fallbacks. Tests cover routing, blank IDs, empty configurations, and first-claimant precedence. ChangesPoster Drive routing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/modules/poster_self_heal.py (1)
47-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the
drive_twinsdocstring to its routing contract.This docstring contains implementation history and failure details. Keep one or two lines that state the resolver order.
Proposed change
- """``(drive_ids, twin_of)`` for the configured ``gdrive_uploads``. - - ``twin_of(image_type)`` gives the Drive folder a locally-saved row of that - type should be renamed in. Art types route to their own folders, so a logo - must heal against the logo Drive — renaming it in the poster Drive raises - (the file isn't there) and the row then never heals at all. First claimer - wins, matching ``_drive_targets``' config-order preference; a type no Drive - claims falls back to the poster Drive, else the first Drive at all (the - pre-redesign behaviour of healing the linked folder even with uploads off). - """ + """Return Drive IDs and an image-type Drive resolver. + + The resolver uses the first claimant, then the poster Drive, then the first Drive. + """🤖 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 `@backend/modules/poster_self_heal.py` around lines 47 - 56, Shorten the docstring describing drive_twins so it only states the routing contract: resolve using the configured upload drives in configuration order, prefer the drive matching the image type, and fall back to the poster drive or first configured drive when needed. Remove implementation history, failure details, and healing behavior explanations.Source: Path instructions
🤖 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 `@backend/modules/poster_self_heal.py`:
- Around line 47-56: Shorten the docstring describing drive_twins so it only
states the routing contract: resolve using the configured upload drives in
configuration order, prefer the drive matching the image type, and fall back to
the poster drive or first configured drive when needed. Remove implementation
history, failure details, and healing behavior explanations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 054bef5e-508c-4cd1-941d-17a3f96bbffc
📒 Files selected for processing (2)
backend/modules/poster_self_heal.pytests/test_poster_self_heal_resolver.py
The rationale and failure mode belong in the PR body, not the source.
Owner
Author
|
Trimmed in 995423b — the docstring now states just the resolver order (first claimant → poster Drive → first Drive). Agreed on the principle: the failure mode and the history belong in the PR body, which already carries them. For the record, the repo style rule the path instructions are enforcing is the right call here — it was the one place this change duplicated prose that already exists above. |
10 tasks
10 tasks
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
poster_self_healpaired every locally-saved row with a single Drive id — the firstgdrive_uploadsentry claimingposter. That was fine when one entry took posters and another took all the art, but now that each art type routes to its own folder, a generatedlogo/background/squareartis renamed against the poster Drive, where the file does not exist.rclone movetoexits non-zero,apply_proposalraises before reaching the local rename, and the row never heals — silently, on every scheduled run. This matters for anyone publishing an artwork drive: downstream users syncing it never receive the corrected filenames.The twin is now resolved from the Drive claiming the row's own
image_type.Related issue
N/A
Type of change
Fallback order is preserved exactly: a type no Drive claims still resolves to the poster Drive, and with no poster Drive at all it resolves to the first configured Drive — the pre-redesign behaviour of healing the linked folder even with uploads off. A single-Drive or uploads-off setup therefore behaves identically to before.
Only the local source was affected. Live-Drive rows already carried the folder they were listed from, so a Drive-only setup was never broken.
Testing
ruff check .— cleanpython -m pytest— 1562 passed (5 new)npm run lint/prettier— untouched, no frontend changeThe mapping is extracted as
drive_twins()so it can be tested directly rather than only throughrun(). The new tests were verified to fail against the previous behaviour — regressingtwin_ofto return the single fallback produces:Covered: per-type routing, fallback for an unclaimed type, fallback when nothing claims posters, the no-Drives case, and that a blank
folder_idcontributes nothing while the first claimer of a duplicated type wins.Screenshots
N/A — no UI change.
Checklist
backend/util/config.py) — N/A, reads existinggdrive_uploadsDAPSreferences introduceddrive_twins()carries the rationale in its docstringmainonlyBoth changed files are develop-only additions, so the branch-isolation invariant holds.
Summary by CodeRabbit
Bug Fixes
Tests