fix(lint): format scripts and add per-file ignores for E402

[cculb]

• Add ruff per-file-ignores config for scripts/*.py to allow imports
  after sys.path modification (necessary for local package imports)
• Format all scripts files to comply with ruff formatting rules

[claude]

PR Review: fix(lint): format scripts and add per-file ignores for E402

Summary

This PR adds a ruff configuration exception for scripts to allow imports after sys.path modification (E402) and reformats all script files to comply with ruff's formatting rules. The changes are focused, well-scoped, and address a legitimate linting issue.

---

✅ Code Quality & Best Practices

Excellent:

• Focused scope: Changes are limited to formatting and configuration, no functional modifications
• Conventional commit format: Title follows type(scope): description pattern correctly
• Consistent formatting: All scripts now follow ruff's formatting standards (line breaks, indentation)
• Proper use of per-file-ignores: The E402 exception is correctly scoped to only scripts/*.py

Good rationale for E402 exception:
The scripts need to modify sys.path before importing local packages (from src.scraper.auth import ...), which is a valid pattern for standalone scripts. The comment clearly explains why the exception is needed.

---

🔍 Potential Issues

None identified - This is a low-risk formatting change.

---

⚡ Performance Considerations

No impact - These are purely cosmetic formatting changes with no runtime performance implications.

---

🔒 Security Concerns

None - No security-relevant changes in this PR.

---

🧪 Test Coverage

Status: CI checks are currently pending (Lint & Type Check, Unit Tests)

Observations:

• The changes should pass linting since that's the whole point of the PR
• No functional code changes, so unit tests should remain passing
• No new tests needed for formatting changes

Per CLAUDE.md requirements:

• This PR contains configuration and formatting changes only
• Once CI passes, no additional validation required (no scraper/MCP/UI changes)
• CI will automatically run lint + unit tests

---

📋 Recommendations

1. ✅ Wait for CI to complete - Let the pending checks finish to confirm linting passes
2. ✅ Verify formatting consistency - The diff shows proper ruff formatting applied consistently
3. ✅ Merge when green - Once CI passes, this is safe to merge

---

🎯 Overall Assessment

LGTM with minor note ✓

This is a clean, well-executed formatting fix. The E402 exception is appropriately scoped and documented. All formatting changes follow ruff's standards consistently.

Score: 9/10

• Clean, focused changes
• Proper tool configuration
• No functional risk
• Follows project conventions

Suggested action: Approve and merge once CI checks pass.

---

Review conducted per CLAUDE.md guidelines. Conventional commits ✓, focused scope ✓, proper validation approach ✓

[claude]

Code Review - PR #31: Fix lint formatting for scripts

Summary

This PR addresses linting issues in the scripts directory by:

1. Adding a per-file ignore for E402 (module-level imports not at top of file) for scripts
2. Reformatting all script files to comply with ruff's formatting rules

✅ Strengths

1. Correct E402 Handling
The E402 ignore is properly justified. Scripts need to modify sys.path before importing local packages:

sys.path.insert(0, str(Path(__file__).parent.parent))
from src.scraper.auth import get_base_url  # This causes E402, but is necessary

Using per-file ignores (scripts/*.py) is the right approach - more targeted than a blanket ignore.

2. Consistent Formatting
All formatting changes follow ruff's rules consistently:

• Multi-line list/dict formatting improved
• Line wrapping for long strings
• Proper indentation in nested structures

3. No Functional Changes
The changes are purely cosmetic - no logic alterations, which reduces risk.

---

🔍 Code Quality Observations

Positive:

• The per-file ignore is well-documented with a comment explaining why it's needed
• Formatting changes improve readability (e.g., multiline dict comprehensions are easier to read)
• Changes are atomic and focused on a single concern

Minor Observations:

1. pyproject.toml:82 - The ignore is correctly scoped to just scripts, but consider if any other files need sys.path manipulation. If none, this is fine.

2. Alternative Approach (Future Consideration):
   Instead of sys.path manipulation, you could make scripts proper entry points by:

   • Moving them to src/cli/ as subcommands
   • Installing the package in editable mode (pip install -e .)
   • Using python -m src.scripts.scrape_full instead

   However, this would be a larger refactor and the current approach is perfectly valid.

---

🐛 Potential Issues

None identified. All changes are safe formatting improvements.

---

🔒 Security Considerations

No security concerns. Formatting-only changes don't affect security posture.

---

⚡ Performance Considerations

No performance impact. Code formatting has zero runtime effect.

---

🧪 Test Coverage

Observation: This is a linting fix, so no new tests are needed. However, verify:

• ✅ CI lint checks should pass after this PR
• ✅ Existing tests should remain unaffected (no functional changes)

Recommendation: Once CI completes successfully, this is ready to merge.

---

📋 Action Items

Before Merge:

• Wait for CI checks to complete and pass
• Verify Lint & Type Check job passes
• Verify Unit Tests still pass (confirms no accidental functional changes)

Optional Future Improvements:

• Consider refactoring scripts to avoid sys.path manipulation (low priority)
• Document the scripts directory structure in README or contributing docs

---

✨ Verdict

LGTM with minor observations.

This is a clean, focused PR that solves the E402 linting issue appropriately. The per-file ignore is well-scoped and documented, and the formatting changes improve code consistency. No blocking issues found.

Recommended action: Merge once CI passes.

---

Review completed by Claude Code Agent

[github-actions]

⚠️ Test Results

Metric	Value
Total Tests	0
Passed	0
Failed	0
Skipped	0

⚠️ No test results found. Check if tests ran correctly.

Test Suite Details

{}

Note: PR checks run unit and integration tests only. E2E tests run on merge to main.

[github-actions]

⚠️ Test Results

Metric	Value
Total Tests	0
Passed	0
Failed	0
Skipped	0

⚠️ No test results found. Check if tests ran correctly.

Test Suite Details

{}

Note: PR checks run unit and integration tests only. E2E tests run on merge to main.