Skip to content

Fix code review issues: dead code, error handling, alignment, DRY defaults#2

Merged
cygnusb merged 1 commit into
mainfrom
code-review-fixes
May 30, 2026
Merged

Fix code review issues: dead code, error handling, alignment, DRY defaults#2
cygnusb merged 1 commit into
mainfrom
code-review-fixes

Conversation

@cygnusb

@cygnusb cygnusb commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Dead code removed: modes/tail.py (run_tail) was never called from cli.py; deleted entirely
  • Config error handling fixed: Bad YAML / invalid config structure now surfaces the real error instead of silently falling through to a misleading --host is required message; adds ConfigFileNotFoundError subclass so only a missing default config path is silently ignored
  • --align wired up in TUI modes: The flag was stored but never passed to MessageLogWidget; now threaded through both GraylogDashboard and LogsOnlyApp
  • Active stream highlighted: StreamsWidget.update_streams accepted active_id but never applied it; now sets lv.index after populating the list
  • Auth errors in throughput fetches trigger exit: _poll previously only called self.exit() for auth errors on the messages fetch; now checks all three results
  • detect_color() removed: Defined in formatter.py but never imported or called
  • except Exception narrowed: JSON parse errors in client.py now catch ValueError instead of bare Exception
  • ThroughputWidget._data uses deque(maxlen=60): Replaces list + manual slice reassignment
  • Shared default constants: DEFAULT_FIELDS, DEFAULT_RANGE_SECONDS, DEFAULT_POLL_INTERVAL_MS extracted to config.py and used across config.py, client.py, and cli.py

Test plan

  • All 37 existing tests pass (uv run pytest)
  • mypy clean (uv run mypy src/)
  • ruff clean (uv run ruff check src/)

🤖 Generated with Claude Code

… defaults

- Remove unused modes/tail.py (run_tail was never called from cli.py)
- Surface config parse errors (bad YAML, invalid structure) instead of silently
  swallowing them; introduce ConfigFileNotFoundError subclass so only a missing
  default config file is allowed to be ignored
- Fix --align flag being silently ignored in TUI modes by threading it through
  MessageLogWidget
- Highlight the pre-selected stream in StreamsWidget.update_streams (active_id
  was accepted but never applied)
- Check all poll results for GraylogAuthError in dashboard _poll, not just messages
- Remove unused formatter.detect_color()
- Narrow except Exception to except ValueError for JSON parse errors in client
- Replace list + manual slice with deque(maxlen=60) in ThroughputWidget
- Extract DEFAULT_FIELDS, DEFAULT_RANGE_SECONDS, DEFAULT_POLL_INTERVAL_MS
  constants to config.py and use them everywhere instead of repeated literals

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cygnusb cygnusb merged commit bc0af33 into main May 30, 2026
3 checks passed
@cygnusb cygnusb deleted the code-review-fixes branch May 30, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant