Skip to content

fix: filter active solvers#7367

Open
limitofzero wants to merge 2 commits intodevelopfrom
fix/allow-only-active-solvers
Open

fix: filter active solvers#7367
limitofzero wants to merge 2 commits intodevelopfrom
fix/allow-only-active-solvers

Conversation

@limitofzero
Copy link
Copy Markdown
Contributor

@limitofzero limitofzero commented Apr 16, 2026

Summary

Remove inactive resolvers from cms

The bug: https://www.notion.so/cownation/Too-many-solvers-3438da5f04ca8065a06cfacf975ef5ba

@limitofzero limitofzero requested a review from elena-zh April 16, 2026 13:03
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cowfi Ready Ready Preview Apr 16, 2026 8:04pm
explorer-dev Ready Ready Preview Apr 16, 2026 8:04pm
swap-dev Ready Ready Preview Apr 16, 2026 8:04pm
widget-configurator Ready Ready Preview Apr 16, 2026 8:04pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
cosmos Ignored Ignored Apr 16, 2026 8:04pm
sdk-tools Ignored Ignored Preview Apr 16, 2026 8:04pm

Request Review

@limitofzero limitofzero requested a review from a team April 16, 2026 13:03
@limitofzero limitofzero self-assigned this Apr 16, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026

Walkthrough

A conditional check was added to filter out inactive solver networks during the mapping process. The code now destructures an active field from entry attributes and skips processing entries where active is false, preventing them from being included in the resulting array.

Changes

Cohort / File(s) Summary
Solver Network Filtering
libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts
Added destructuring of active field from entry.attributes with an early return condition to exclude inactive solver networks from the mapping result.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A filter so neat, with active to check,
Inactive solvers removed—what a tech!
Networks now cleaner, more honest and true,
This mapper hops forward with work shiny new! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The pull request description is incomplete and lacks most required template sections. Add 'To Test' section with step-by-step verification instructions and expected outcomes. Consider adding 'Background' section for context about the bug fix.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: filter active solvers' directly describes the main change: filtering to include only active solvers, matching the core functionality of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/allow-only-active-solvers

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts (2)

8-10: Please remove complexity suppression in this touched mapper

Line 8-9 keeps TODO + eslint-disable scaffolding while adding more branching here. Extract the network-entry mapping/filter into a small helper and drop the suppression.

As per coding guidelines, remove linter scaffolding (// TODO, eslint-disable) and extract helpers instead of disabling complexity checks.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts` around lines 8 -
10, Remove the TODO and the "// eslint-disable-next-line complexity" from
mapCmsSolversInfoToSolversInfo and extract the inner reduce logic that
maps/filter solver_networks.data entries into a new small helper (e.g.,
mapSolverNetworkEntry or filterAndMapSolverNetwork) that returns either a
SolverNetwork or null/undefined for invalid entries; then replace the inline
reduce callback in solverNetworks with a clear usage of the helper (filtering
out falsy results) so the complexity of mapCmsSolversInfoToSolversInfo is
reduced and the linter suppression can be removed.

12-17: Prefer allowlisting active entries (active === true) instead of only skipping false

On Line 15, active === false still passes through entries where active is missing. If the goal is “only active solvers,” use a positive check and return early for everything else.

Suggested change
-          // skip inactive solvers
-          if (active === false) {
+          // allow only active solvers
+          if (!active) {
             return acc
           }

As per coding guidelines, use !!value for explicit boolean conversions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts` around lines 12 -
17, The current filter skips only explicit false values so entries with
missing/undefined active still pass; in mapCmsSolversInfoToSolversInfo replace
the negative check (active === false) with a positive allowlist using an
explicit boolean conversion (e.g., ensure active is strictly true via !!active
or Boolean(active)) and return early for anything else so only entries where
active is true are processed (use the entry.attribute name `active` and
accumulator `acc` to locate the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts`:
- Around line 8-10: Remove the TODO and the "// eslint-disable-next-line
complexity" from mapCmsSolversInfoToSolversInfo and extract the inner reduce
logic that maps/filter solver_networks.data entries into a new small helper
(e.g., mapSolverNetworkEntry or filterAndMapSolverNetwork) that returns either a
SolverNetwork or null/undefined for invalid entries; then replace the inline
reduce callback in solverNetworks with a clear usage of the helper (filtering
out falsy results) so the complexity of mapCmsSolversInfoToSolversInfo is
reduced and the linter suppression can be removed.
- Around line 12-17: The current filter skips only explicit false values so
entries with missing/undefined active still pass; in
mapCmsSolversInfoToSolversInfo replace the negative check (active === false)
with a positive allowlist using an explicit boolean conversion (e.g., ensure
active is strictly true via !!active or Boolean(active)) and return early for
anything else so only entries where active is true are processed (use the
entry.attribute name `active` and accumulator `acc` to locate the change).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a61efcf1-c09b-4a29-9bfe-00cbf2891fdb

📥 Commits

Reviewing files that changed from the base of the PR and between 50434ec and 1bd9cfe.

📒 Files selected for processing (1)
  • libs/core/src/cms/utils/mapCmsSolversInfoToSolversInfo.ts

Copy link
Copy Markdown
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Hey @limitofzero , great!

But this is weird on Arbitrum:

Image

Total number of solvers correspond to the active ones on Explorer, but why then more solvers participated that we have active ones?

Image

Same on Avalanche

Image

Could you please check why?

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.

2 participants