Conversation
… + add rollback calibre_open() returned False (opening the write gate) on any host without pgrep, silently disabling the "refuse while Calibre is open" guard. It now tries pgrep, falls back to ps, and fails CLOSED if neither exists; GUI matching moved to _is_calibre_gui() and tightened to also exclude calibredb/calibre-server/parallel. Backups move from /tmp to data/backups/ (survive reboot, gitignored, pruned to the newest 20). _backup_path() adds an _N suffix so two writes in the same second can no longer overwrite one snapshot — the exact scenario a guided wizard run creates — and run_writer verifies the copy size before writing. New `scourgify rollback [--list]` restores the newest (or a chosen) snapshot with Calibre-closed and valid-Calibre-DB checks, snapshotting the current db first so the rollback is itself reversible. tests/test_backup.py pins the GUI matcher, collision-proof paths, and pruning (added to CI). README/CLAUDE.md updated for the new backup location, rollback command, and fail-closed detection. Closes #4, closes #5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…type hints) Resolves the actionable findings from the desloppify holistic review: - contract/high_level/convention: refresh stale docstrings in common.py and wizard.py (backup path /tmp->data/backups/; landing-menu; no-main() note) - authorization: name the tag mass-deletion thresholds (TAG_SHRINK_FRACTION/FLOOR) - error_consistency: unify engine-error truncation behind ERR_TRUNC; document the select.pick ValueError as an internal programmer-error guard - test_strategy: gate test_promote.py + test_layers.py in CI; add tests for the pure tag_loss_guard and the extracted data_loss_guard safety rails - type_safety: annotate the highest-fan-in seams (norm, ascii_fold, read_custom_column, load_config, transform, select.pick, parse_resp) - mid_level_elegance: compute _engines() once per wizard menu, reuse hints/usable Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A wedged calibre-debug could hang a scripted/CI apply forever. Cap the write at a generous 1h (a real batch write finishes in seconds/minutes), raise a clear SystemExit on TimeoutExpired (backup is already taken), and move the temp-file cleanup into finally so it runs on the timeout path too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cli.py's routing table was the one untested piece of real (non-interactive) logic. Pin every branch — version flag, classify/staleness/promote routing, overrides/rollback cmd handlers, bare/unknown fall-through to wrangle, and the sys.argv reframing the subcommands' argparse relies on. Stub each tool main() with a recorder so it needs no Calibre/library/network. Gate it in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…png) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The advertised "no book loses its last fandom/character" abort could never fire: transform() gated it on `had_real_F and not nF`, but any fandom that made had_real_F true was the same one added to nF, so the condition was a contradiction (always False). Meanwhile a book genuinely CAN end with an empty #fandoms via a bad fandoms.csv alias->"" or an empty decompose payload — and the SAFETY line reassuringly printed "losing last fandom: 0". Fix: flag a loss when a book had a fandom and ends with none, tracking blocklist relocations (value preserved in tags) so those don't count. Give data_loss_guard a --force escape like tag_loss_guard, with a message that points at the likely override. audit's dry-run SAFETY line now shows real counts; apply aborts unless --force. Tests pin that the guard now fires on alias->"" and empty-decompose, stays quiet on relocation/normal routing, and that --force overrides. Fixes #10 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ran bandit 1.9 over src/scourgify: 19 findings, all verified safe for this tool's threat model (single-user local CLI, no network/web input surface). Documented the triage in a [tool.bandit] skips block — subprocess argv are fixed lists to trusted Calibre tools (never shell=True), urlopen hits hardcoded https API literals, SQL uses ? placeholders / schema-derived integer ids, and the calibre_open probe intentionally fails closed. `bandit -c pyproject.toml -r src/scourgify` is now clean, so future scans surface only new issues. Addresses #14 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tag-mass-loss / last-fandom guards lived only in wrangle.apply_changes, so classify --apply / promote --backfill / staleness / setup inherited the auto backup but not the safety checks — "every write is guarded" was per-caller discipline, not structural. run_writer now predicts each set_field's effect (pure _predict_populated) and refuses, BEFORE writing, a change-set that would empty a populated column (_is_wipe: >90% of a >=100-book column). Coarse on purpose — no legitimate write approaches it, so it never false-positives on wrangle's own 25%-capped drops; it exists to catch a future writer silently regressing. --force overrides, and wrangle threads its force through so a deliberate forced deletion isn't double-blocked. Add-only writers (classify/promote/staleness) get it for free. Pure helpers are unit-tested (no DB needed); docs updated. Closes #6 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The char/trope fold maps were raw-keyed, but is_junk, known_chars, and redundancy-strip all match on norm(). A custom-map entry that differed from the book's value only in casing or punctuation folded silently — the user had to guess the exact spelling. load_maps now adds norm-keyed aliases (raw keys still win) and every char/trope lookup goes through _lookup (exact, then norm). Refs #9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…to overrides.py Pure move, no logic changes. wrangle.py owned two concerns — the core normalize/audit/apply engine and the whole reject→overrides subsystem behind `apply --step` / `scourgify overrides`. The seam was already clean (cli.py dispatches `overrides` separately; _step_walk takes the classified edits as a param), so the cluster (_edit_label, reconstruct, synth_reject, _reject_row, _step_walk, _append_override, build_overrides, _archive_consumed_rejects, overrides_cmd) moves to a sibling module. The wrangle<->overrides cycle is broken by a lazy import inside apply_changes; overrides imports read_csv/read_lines/transform from wrangle at module level. cli/wizard/tests repointed. wrangle.py 817 → 618 LOC. Full suite green. Closes #11. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the type hints from the five high-fan-in seams to the rest of the public functions in common, select, staleness, promote, classify — plus the two largest engine modules (wrangle, overrides) so type_safety improves holistically, not just in the modules the issue named. Plain honest hints (dict / list / tuple / X | None / argparse.Namespace), no typing gymnastics; this also clears the 'loose type annotation' smells. No behavior change; full suite green. Refs #12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion helpers wizard.py and ui.py were the last untested production files. Test only the pure helpers where a regression can actually ship: _task_hint (the pending-work menu markers), _engines (cloud-engine-usable-iff-key-in-env + hints), and the proposal pending/to_stamp split — extracted from snapshot() into a pure _proposal_counts() so it's testable without a live library. The rich-interactive shells (menus, prompts, live dashboard) are left untested by design. Closes #13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…fill + docs Three verified polish items from the #9 batch: - --bakeoff: the same sample-books-across-engines comparison the wizard offers is now available as `scourgify classify --bakeoff` (display-only, writes nothing). Engine metadata (ENGINE_ENV + usable_engines) now lives in classify as the single source of truth; wizard.ENGINE_KEYS aliases it so the two can never disagree. - backfill preview now shows each book's title, not just its raw id. - README engine list includes mistral (it was already in PRICING/ENGINE_ENV/classify). Closes #9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… freeforms The curated vocab was ~120 terms while wrangle ships the full ~93k-fold AO3 taxonomy, so the classifier under-tagged out of the box (issue #8). The highest-use AO3 freeforms ARE the generic reusable tropes (Fluff, Angst, Slow Burn, Hurt/Comfort…) — a use-count threshold cleanly separates them from fandom-specific noise, which never accumulates many uses. build_classify_seed.py (maintainer tool) reads data/ao3_vocab.csv (name,uses; from the OTW dump via build_ao3_layer.py), gates at >=20k uses, drops fandom-specific '(...)' and format/meta tags, and writes the regenerable defaults/classify_vocab_ao3.txt (~120 terms). load_vocab() now merges curated core ∪ AO3 seed (dedup-on-merge, curated spelling wins), still fully overridable. Also removes a pre-existing 'Found Family' dup the merge-dedup test surfaced. Closes #8. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ecipe The bundled defaults/ao3/ layer is generated from the only public OTW dump (2021-02-26), so franchises that blew up after it (Baldur's Gate 3, Elden Ring, Honkai: Star Rail) aren't in the master taxonomy and won't auto-fold on a fresh install. The full fix is a regen against a newer OTW dump — external, none released yet — so this documents the staleness prominently and gives a concrete overrides/ recipe (fandoms.csv + characters.csv) plus the promote-loop alternative. Addresses the user-facing half of #7; full AO3 regen stays blocked on a newer source dump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
Release v1.2.0 — the desloppify code-health roadmap (strict score 82.3 → 83.9) plus classify usability.
Highlights since v1.1.0
Safety
run_writernow refuses a change-set that would catastrophically empty a populated column (coarse last-line wipe guard covering every writer;--forceoverrides).calibre_open()fails closed when Calibre is undetectable; backups hardened;run_writersubprocess has a timeout.classify
scourgify classify --bakeoff— compare sample books across every usable engine, then exit.build_classify_seed.py→defaults/classify_vocab_ao3.txt), so it isn't under-tagging out of the box;mistralengine documented.Engine
--stepreject→overrides subsystem is split into its ownoverrides.py(wrangle.py 817→618 LOC).Quality
type_safetyre-review 62→82).test_cli,test_wizard; wired into CI. Reviewed bandit config.Merge commit (not squash) per the release flow; auto-publishes to TestPyPI on landing.
🤖 Generated with Claude Code