E4/E5: lint gates (ruff+SwiftFormat) and README screenshots#66
Merged
Conversation
Conservative rule set (preserve existing wrap/collection style, 4-space indent, 120-col max) chosen to minimize churn while still catching real issues (trailing whitespace, inconsistent brace spacing, etc). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
One-time reformat -- the codebase never had a formatter run against it. Purely mechanical (verified: builds clean, all 108 Swift tests pass, no logic changes). Adding the commit SHA to .git-blame-ignore-revs next so `git blame` still resolves through this reformat. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ruff gate widened from a narrow error-tier subset (E9,F63,F7,F82) to the full configured rule set, now that the 288 pre-existing violations are fixed. Adds a SwiftFormat --lint step to the macOS smoke job. Also adds .git-blame-ignore-revs so `git blame` skips the one-time SwiftFormat reformat commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Auto-fixed unused imports/f-strings/unused-variables where safe (verified each removal against real usage, not just ruff's say-so), then manually resolved the categories ruff won't autofix: - E712: assert x == True/False -> assert x / assert not x (all against plain-bool-returning predicates, no numpy ambiguity) - E701/E702: split multi-statement lines (if x: y, a=1; b=2) onto separate lines - E402: moved genuinely-misplaced imports to the top; kept sys.path-fixup imports where the ordering is load-bearing, marked with noqa + reason - B904: chained exceptions with `from e` where the original error is useful context, `from None` where the wrapper message is self-contained - B007: renamed/removed unused loop-control variables - E722: bare `except:` -> `except Exception:` - B017: kept broad `pytest.raises(Exception)` where multiple exception types are legitimately expected (malformed-input corpus tests), with noqa + reason rather than fabricating a narrower type - B023/E731: fixed a real loop-variable-closure risk in docx_io.py and a lambda-assignment in native_setup.py Full suite verified after every batch (555 passed, 6 skipped throughout). One incidental finding spun off separately rather than fixed here: a dead expression removed in test_docx_io.py exposed a real test-coverage gap (only one direction of a bidirectional field-mapping check was wired up). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The test only checked that every CLI_ARG_MAP entry maps to a real settings field, not the reverse -- a new bool field on MetadataCleaningSettings could ship without a CLI arg mapping and nothing would catch it. Surfaced by dead-code removal during the ruff cleanup (a computed-but-never-asserted set from an earlier, incomplete attempt at this same check). Verified the new assertion has teeth: temporarily injected an unmapped bool field, confirmed the test fails with a clear message naming the field, then reverted the injection. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
assets/help.md (the documented canonical source, per CLAUDE.md) had gone stale relative to src/swift/MarcutApp/Sources/MarcutApp/Resources/help.md: PR #65's doc sweep (df99203) edited the Swift-side copy directly instead of the canonical source, so assets/help.md never got the new pre-flight checks, streaming-progress, fail-closed-extraction, and sleep/wake sections. scripts/render_help_html.py (invoked by scripts/sh/build_swift_only.sh) treats assets/help.md as authoritative and regenerates the Swift copy from it on every build -- so building after PR #65 silently reverted the in-app help text, which is how this was discovered (while producing a build for E5 README screenshots). Fix: copied the current (good) content back to assets/help.md so `python3 scripts/render_help_html.py` is now a no-op against both copies -- verified via diff and by re-running the script. Added a CI check (regenerate from assets/*.md, fail if the tracked Swift Resources/*.md copies then show a git diff) so this exact regression class can't land silently again. Verified the check both ways: passes clean on the current state, and correctly fails when the old stale assets/help.md content is re-injected (tested, then reverted). Only the .md fallback copies are compared -- Resources/*.html is gitignored generated output, not tracked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Built a real, ad-hoc-signed app bundle (scripts/sh/build_swift_only.sh preset dev_fast + create_dmg), installed it, and captured 4 screenshots of actual running states against a purely synthetic demo NDA (fabricated names/companies/.example addresses -- never real sample-files content): idle drag-and-drop, live AI-analysis progress, finished-processing status, and the redaction settings panel. Along the way: found the raw SwiftPM debug binary crashes on launch (UNUserNotificationCenter asserts when called from an unbundled/unsigned binary -- fixed by building a real .app via the project's own dev_fast preset instead); found computer-use's screenshot save-to-disk isn't reachable from this shell's filesystem, so screenshots were captured directly via screencapture(1) once Screen Recording permission was granted, then cropped to the app window via a color-based bounding-box detection (Pillow) rather than guessed pixel coordinates. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The macOS CI runner image ships SwiftFormat 0.61.1 pre-cached, so the `brew install swiftformat` step was a no-op and never picked up the 0.62.1 I tested locally -- CI failed on 4 real violations 0.62.1 doesn't flag: redundant explicit `Sendable` conformance on two non-public enums (DocumentModels.swift), a comment style mismatch, and a string-literal indent/trailing-space conflict inside a multi-line alert message that 0.61.1's indent and trailingSpace rules disagreed on for blank lines -- resolved by using explicit `\n\n` instead of literal blank lines in the string, sidestepping the whole blank-line-in-multiline-string ambiguity. Verified all fixes clean under both 0.61.1 and 0.62.1 (downloaded both release binaries directly rather than trusting brew's ambient version). CI now pins to a specific downloaded release (0.62.1) instead of whatever the runner image happens to have cached, so this can't silently drift again. Co-Authored-By: Claude Sonnet 5 <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.
Summary
NameErrorcrash bug ingui.py's download-error dialogs (caught by the new ruff gate), and staleassets/help.mdvs the Swift Resources copy (now resynced, with a CI check added so it can't drift again silently).test_all_fields_have_cli_args(only checked one direction of the CLI-arg mapping).Test plan
PYTHONPATH=src/python python3 -m pytest -q— 555 passed, 6 skippedswift test --package-path src/swift/MarcutApp— 108 passedruff check src/python tests— cleanswiftformat --lint src/swift/MarcutApp/Sources src/swift/MarcutApp/Tests— cleanassets/help.mddrift verified both ways (passes clean, fails on injected stale content)🤖 Generated with Claude Code