Skip to content

chore: remove bullmq - #453

Merged
coodos merged 4 commits into
mainfrom
chore/remove-bull-mq
Nov 20, 2025
Merged

coodos merged 4 commits into
mainfrom
chore/remove-bull-mq

Conversation

@coodos

@coodos coodos commented Nov 20, 2025 •

Copy link
Copy Markdown
Contributor

Description of change

Remove BullMQ

Issue Number

Type of change

  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • Chores

    • Removed unused queue-related dependencies and background worker components.
  • Improvements

    • eReputation processing now runs inline/synchronously, improving response time for reputation-weighted polls.
    • The system automatically generates and persists detailed reputation outcome messages for tracked polls.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 20, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Removed the BullMQ-based job queue and worker; WebhookController now performs eReputation weighted-poll processing synchronously using VotingReputationService and MessageService. package.json deps bullmq and ioredis removed and JobQueueService/PollReputationWorker modules deleted.

Changes

Cohort / File(s) Summary
Dependency removal
platforms/eReputation-api/package.json
Removed bullmq and ioredis from dependencies.
Controller refactor — inline processing
platforms/eReputation-api/src/controllers/WebhookController.ts
Removed jobQueueService and DI constructor parameter; added votingReputationService and messageService fields. Constructor now parameterless and instantiates services. Added private methods processEReputationWeightedPoll, transmitReputationResults, and createReputationMessage. Replaced enqueue calls with direct processing and local error handling.
Queue/service removal
platforms/eReputation-api/src/services/JobQueueService.ts
Entire JobQueueService module removed (including PollReputationJobData iface and methods enqueuePollReputationJob, close, getQueue).
Worker removal
platforms/eReputation-api/src/workers/PollReputationWorker.ts
Removed PollReputationWorker and its worker-based processing, including transmit and message routines.
Initialization updates
platforms/eReputation-api/src/index.ts
Removed imports/instantiation of JobQueueService and PollReputationWorker; instantiate WebhookController() with no args.

Sequence Diagram(s)

sequenceDiagram
    participant Webhook as Webhook
    participant Controller as WebhookController
    participant VRS as VotingReputationService
    participant MS as MessageService
    participant Adapter as External Adapter

    rect rgb(200,220,255)
    note over Controller: Old (removed)
    Webhook->>Controller: handlePoll()
    Controller->>JobQueue: enqueuePollReputationJob()
    JobQueue->>Worker: async process (worker handled reputation & messaging)
    end

    rect rgb(220,255,220)
    note over Controller: New (inline)
    Webhook->>Controller: handlePoll()
    Controller->>Controller: processEReputationWeightedPoll()
    Controller->>VRS: computeReputation(poll)
    VRS-->>Controller: reputationResults
    Controller->>Controller: transmitReputationResults()
    Controller->>Adapter: sync results
    Adapter-->>Controller: ack
    Controller->>Controller: createReputationMessage()
    Controller->>MS: persist system message
    MS-->>Controller: created
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • Correctness and robustness of the three new async methods in WebhookController (data loading, validation, error handling).
    • Any transaction/consistency guarantees previously provided by the worker/queue (retries, deduplication) — ensure equivalents or acceptable risk.
    • Service instantiation in constructor vs prior dependency injection — verify configuration, lifecycle, and test coverage.
    • Removal of JobQueueService and worker: ensure no remaining references and that performance/timeout implications are addressed.

Poem

🐰 I swapped the queue for nimble feet,
Inline I hop — results are neat.
Scores computed, messages made,
No worker burrow in the glade.
Hooray — reputation done on beat! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete with several required sections left blank: Issue Number is empty, testing details are missing, and all checklist items are unchecked without any explanation of completion status. Complete the description by filling in the Issue Number, providing specific testing details in the 'How the change has been tested' section, and checking off or explaining the status of each checklist item.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: remove bullmq' accurately summarizes the main change: removing BullMQ dependencies and related job queue infrastructure from the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/remove-bull-mq

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83b696d and 220474f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • platforms/eReputation-api/src/services/JobQueueService.ts (0 hunks)
  • platforms/eReputation-api/src/workers/PollReputationWorker.ts (0 hunks)
💤 Files with no reviewable changes (2)
  • platforms/eReputation-api/src/workers/PollReputationWorker.ts
  • platforms/eReputation-api/src/services/JobQueueService.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: lint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coodos
coodos marked this pull request as ready for review November 20, 2025 11:46

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
platforms/eReputation-api/src/controllers/WebhookController.ts (3)

268-273: Fire‑and‑forget eReputation processing: behavior and reliability trade‑offs

Triggering processEReputationWeightedPoll(...) without await and only logging errors gives you non-blocking webhooks, but also:

  • No retry/backoff if OpenAI/DB fails.
  • No deduping/idempotency if the same poll is processed multiple times, which can emit repeated system messages and adapter updates.
  • Work is lost if the process dies mid‑calculation (previously handled by the queue).

If you want to keep things simple but harden behavior, consider:

  • Adding a lightweight idempotency guard keyed by poll.id (e.g., short‑lived in‑memory set) to avoid concurrent duplicate runs.
  • Optionally wrapping this in a small internal “job runner” abstraction so you can later layer retries/metrics without re-touching controller logic.

Also applies to: 298-303


335-395: Minor cleanup: unused parameters and potential simplification

transmitReputationResults reloads VoteReputationResult, Poll, and Group from the DB and never uses the poll and group parameters passed in. That’s harmless but slightly confusing.

Two small cleanups you might consider:

  • Drop the poll/group parameters from the signature, or actually use them to avoid some reloads.
  • If adapter.handleChange accepts structured JSON, you may not need JSON.stringify(reloadedResult.results) (keep as‑is if the adapter explicitly expects a string).

These are cosmetic and can be deferred.


397-424: System message generation is sound; consider batching user lookups

The message text assembly and error handling look good and will produce a clear, human‑readable summary.

If groups can be large, you might later refactor the per‑user await this.userService.getUserByEname(...) loop into a batched query (or Promise.all) to cut down on N+1 DB hits, but it’s fine to leave as‑is for now.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between edd8023 and 83b696d.

📒 Files selected for processing (3)
  • platforms/eReputation-api/package.json (0 hunks)
  • platforms/eReputation-api/src/controllers/WebhookController.ts (5 hunks)
  • platforms/eReputation-api/src/index.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • platforms/eReputation-api/package.json
🧰 Additional context used
🧬 Code graph analysis (2)
platforms/eReputation-api/src/index.ts (1)
platforms/eReputation-api/src/controllers/WebhookController.ts (1)
  • WebhookController (16-425)
platforms/eReputation-api/src/controllers/WebhookController.ts (1)
platforms/eReputation-api/src/services/VotingReputationService.ts (1)
  • VotingReputationService (8-506)
🔇 Additional comments (2)
platforms/eReputation-api/src/index.ts (1)

55-55: Controller wiring matches new constructor signature

Instantiating WebhookController without arguments aligns with the new no-arg constructor and removal of the queue service; nothing else needed here.

platforms/eReputation-api/src/controllers/WebhookController.ts (1)

314-333: Inline eReputation pipeline is coherent and well‑guarded

This helper cleanly:

  • Bails out when groupId/group.charter are missing.
  • Delegates calculation + persistence to VotingReputationService.
  • Separates concerns by delegating transmission and messaging.

The control flow and guards look correct for the updated design.

Comment on lines +6 to +7
import { VotingReputationService } from "../services/VotingReputationService";
import { MessageService } from "../services/MessageService";

@coderabbitai coderabbitai Bot Nov 20, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Eagerly constructing VotingReputationService can make OPENAI mandatory for the API to start

Because VotingReputationService’s constructor throws when OPENAI_API_KEY is missing, new WebhookController() will now fail at process startup in environments that don’t configure that key (e.g., local/dev or tests that don’t need eReputation). Previously, this dependency was likely isolated to the worker/queue path.

Consider either:

  • Lazily instantiating VotingReputationService inside processEReputationWeightedPoll (on first use), or
  • Making the service tolerate a missing key and no-op eReputation, so the rest of the API can still boot.

This keeps the “remove bullmq” goal while avoiding a new hard startup dependency on OpenAI.

Also applies to: 12-12, 21-22, 25-25, 30-31

🤖 Prompt for AI Agents
In platforms/eReputation-api/src/controllers/WebhookController.ts (lines 6-7,
12, 21-22, 25, 30-31): the controller currently eagerly constructs
VotingReputationService which throws if OPENAI_API_KEY is missing, preventing
app startup; change to lazily instantiate VotingReputationService only inside
processEReputationWeightedPoll (create on first use) or update the service to
detect a missing OPENAI_API_KEY and become a no-op implementation (return early
or mock methods) so construction won’t throw; ensure all places that reference
the eager instance use the lazily-created instance (guard with a null check and
create if undefined) and add a clear comment about the lazy/no-op behavior so
non-eReputation environments can boot without OpenAI configured.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nack, works as intended

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coodos, understood! Thanks for clarifying that the eager instantiation and the OPENAI_API_KEY requirement at startup are intentional design decisions.


✏️ Learnings added
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 453
File: platforms/eReputation-api/src/controllers/WebhookController.ts:6-7
Timestamp: 2025-11-20T12:00:02.259Z
Learning: In platforms/eReputation-api/src/controllers/WebhookController.ts, the eager instantiation of VotingReputationService in the constructor (which requires OPENAI_API_KEY at startup) is intentional behavior and works as intended.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@coodos
coodos merged commit 94ed464 into main Nov 20, 2025
4 checks passed
@coodos
coodos deleted the chore/remove-bull-mq branch November 20, 2025 12:02
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