Skip to content

fix: comprehensive bug fixes and documentation updates — round 17#258

Merged
SimplicityGuy merged 2 commits intomainfrom
bugfix/round-17-comprehensive
Apr 2, 2026
Merged

fix: comprehensive bug fixes and documentation updates — round 17#258
SimplicityGuy merged 2 commits intomainfrom
bugfix/round-17-comprehensive

Conversation

@SimplicityGuy
Copy link
Copy Markdown
Owner

Summary

  • 11 bug fixes across API, consumer services, shared library, Rust extractor, scripts, and utilities
  • 19 documentation files updated for accuracy against actual codebase state

Bug Fixes

Critical

  • api/metrics_collector.py: extractor-musicbrainz health port was 8011 (brainzgraphinator's port) instead of 8000 — dashboard showed wrong health data
  • api/dependencies.py: require_admin was missing password-changed revocation check — admin tokens survived password resets
  • api/api.py: OAuth getdel made retry impossible on exchange failure — changed to get + post-success delete
  • brainzgraphinator/brainzgraphinator.py: Message counts incremented before processing/ack — inflated progress on retries

Important

  • common/postgres_resilient.py: AsyncPostgreSQLPool.initialize() TOCTOU race — two coroutines could race, overwriting the connection queue
  • common/state_marker.py: Lock scope violation — unprotected reads after lock release in complete_processing()
  • extractor/src/state_marker.rs: start_download() didn't clear downloads_by_file map — counter drift on resume

Scripts/Tooling

  • scripts/test-database-resilience.sh: Queue names missing discogs- source segment (always returned 0); monolithic extractor service name
  • utilities/check_queues.py: Only monitored graphinator queues — now monitors all consumers

Documentation Updates

Recurring fixes across 19 files:

  • MusicBrainz release-groups exchange consistently omitted (3→4 everywhere)
  • Monolithic extractor references → extractor-discogs/extractor-musicbrainz
  • Incomplete health endpoint lists → all 9 services included
  • Line length 88→150 in dev/contributing docs
  • Missing services from coverage, logging, Dockerfile standards, maintenance procedures
  • Architecture diagrams updated to show 8 total exchanges

Test plan

  • All 2743 Python tests pass (1862 API/common + 881 other services)
  • Rust extractor compiles (cargo check clean)
  • All pre-commit hooks pass (ruff, mypy, bandit, shellcheck, clippy, cargo fmt)

🤖 Generated with Claude Code

Bug fixes (11 issues):
- Fix extractor-musicbrainz health port collision in metrics collector (8011→8000)
- Add missing password-changed revocation check to require_admin dependency
- Fix OAuth verify flow: use get+delete instead of getdel to allow retry on exchange failure
- Move brainzgraphinator message count increment after successful ack
- Fix AsyncPostgreSQLPool.initialize() TOCTOU race with dedicated init lock
- Fix lock scope violation in Python state_marker.complete_processing()
- Clear downloads_by_file in Rust start_download() to prevent counter drift
- Fix queue names in test-database-resilience.sh (add discogs- source segment)
- Fix monolithic extractor Docker service name in resilience test script
- Expand check_queues.py to monitor all consumer queues, not just graphinator

Documentation updates (19 files):
- Fix MusicBrainz exchange count from 3→4 (add release-groups) across all docs
- Replace monolithic 'extractor' references with extractor-discogs/extractor-musicbrainz
- Complete health endpoint lists (add brainzgraphinator, brainztableinator, insights)
- Fix line length from 88→150 chars in development.md and contributing.md
- Update coverage source list in testing-guide.md
- Add musicbrainz schema to PostgreSQL maintenance procedures
- Add missing services to dockerfile-standards.md port/requirements sections
- Update architecture diagrams to show 8 total exchanges

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

… double-check

- Add test_password_changed_revokes_admin_token: verifies require_admin rejects
  tokens issued before a password change (covers lines 108-113 in dependencies.py)
- Add test_password_changed_allows_newer_admin_token: verifies tokens issued after
  password change are allowed through
- Add test_do_initialize_double_check_skips_when_already_initialized: exercises
  _do_initialize() double-check locking path in AsyncPostgreSQLPool

Codecov patch coverage: dependencies.py 57% → 100%, postgres_resilient.py 87% → 99%

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@SimplicityGuy SimplicityGuy merged commit c099b68 into main Apr 2, 2026
41 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

E2E Coverage (webkit)

Totals Coverage
Statements: 40.99% ( 1081 / 2637 )
Lines: 40.99% ( 1081 / 2637 )

StandWithUkraine

@SimplicityGuy SimplicityGuy deleted the bugfix/round-17-comprehensive branch April 2, 2026 00:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

E2E Coverage (firefox)

Totals Coverage
Statements: 40.99% ( 1081 / 2637 )
Lines: 40.99% ( 1081 / 2637 )

StandWithUkraine

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

E2E Coverage (chromium)

Totals Coverage
Statements: 40.99% ( 1081 / 2637 )
Lines: 40.99% ( 1081 / 2637 )

StandWithUkraine

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

E2E Coverage (webkit - iPhone 15)

Totals Coverage
Statements: 40.99% ( 1081 / 2637 )
Lines: 40.99% ( 1081 / 2637 )

StandWithUkraine

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

E2E Coverage (webkit - iPad Pro 11)

Totals Coverage
Statements: 40.99% ( 1081 / 2637 )
Lines: 40.99% ( 1081 / 2637 )

StandWithUkraine

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