feat: add --dry-run flag to boost-scores.js#240
Merged
ritik4ever merged 7 commits intoJun 29, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesBoost-scores dry-run and export refactor
CI workflow updates
Sequence Diagram(s)sequenceDiagram
participant CLI
participant boost
participant recordPaymentOnChain
CLI->>boost: boost({ dryRun, serviceId })
boost->>boost: validate serviceId in live mode
alt dryRun
boost->>CLI: log planned payments, skip chain calls
else live mode
boost->>recordPaymentOnChain: recordPaymentOnChain(agent, serviceId, amount, flag)
boost->>CLI: log completion
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/scripts/boost-scores.test.js (1)
40-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the full dry-run log contract.
The PR requires dry-run logs for current score, target, needed payments, and total amount; these assertions only check
nameandpayments.🧪 Proposed test strengthening
expect(logger.info).toHaveBeenCalledWith( - expect.objectContaining({ name: 'agent-1', payments: 1 }), + expect.objectContaining({ + name: 'agent-1', + currentScore: 100, + target: 110, + payments: 1, + totalAmount: '10000', + }), '[dry-run] Would submit on-chain payments', ); @@ expect(logger.info).toHaveBeenCalledWith( - expect.objectContaining({ name: 'agent-2', payments: 10 }), + expect.objectContaining({ + name: 'agent-2', + currentScore: 500, + target: 600, + payments: 10, + totalAmount: '100000', + }), '[dry-run] Would submit on-chain payments', );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/boost-scores.test.js` around lines 40 - 48, The dry-run assertions are only checking a partial log payload; update the test around the logger.info calls in the payment boost flow to assert the full dry-run contract, including current score, target score, needed payments, and total amount alongside the agent name. Use the existing logger.info expectations in this test to verify the complete object shape emitted by the dry-run path so the contract in the boost-scores script is fully covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/scripts/boost-scores.js`:
- Line 9: The live-mode call to recordPaymentOnChain in boost is missing the
serviceId argument, causing the remaining parameters to shift and corrupt the
on-chain payment record. Update the boost flow so the call passes agentAddress,
serviceId, amountStroops, and success in the correct order, using the service id
from the target/payment context alongside the existing amount and status values.
- Line 60: The direct-run guard in boost-scores.js is using a brittle string
comparison against import.meta.url, which can fail for paths with URL-encoded
characters. Update the isDirectRun check to compare process.argv[1] against a
file-system path derived from import.meta.url via fileURLToPath(), and keep the
CLI entry condition in the same top-level guard so the main body still runs when
invoked directly.
In `@backend/scripts/boost-scores.test.js`:
- Around line 75-79: The live-mode test for boost() is asserting the wrong call
shape for recordPaymentOnChain, which now expects agentAddress, serviceId,
amountStroops, and success. Update the boost-scores test to pass a serviceId
into boost() and assert recordPaymentOnChain with the explicit service id in
addition to GTESTADDRESS, 10_000n, and true so the mocked call matches the
four-argument contract.
---
Nitpick comments:
In `@backend/scripts/boost-scores.test.js`:
- Around line 40-48: The dry-run assertions are only checking a partial log
payload; update the test around the logger.info calls in the payment boost flow
to assert the full dry-run contract, including current score, target score,
needed payments, and total amount alongside the agent name. Use the existing
logger.info expectations in this test to verify the complete object shape
emitted by the dry-run path so the contract in the boost-scores script is fully
covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ce8f8b6-6b1c-4732-8e04-f0e3f6475caf
📒 Files selected for processing (2)
backend/scripts/boost-scores.jsbackend/scripts/boost-scores.test.js
Contributor
Author
CI StatusAll 4 CI checks failed during the setup phase — none of the failures are related to the code changes in this PR.
Backend/Frontend/Agent never reached the test execution step — they all failed at Node.js setup. The contract job fails because of a Happy to re-run once CI infra is fixed. The boost-scores changes and tests themselves work locally (3/3 pass). |
This was referenced Jun 28, 2026
Contributor
Author
|
Will look into it. Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend/lib/sort.test.ts (1)
93-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the changed tie-breaker branches too.
sortAgentsWithTieBreakernow changed itspaymentsandnewestcomparisons, but this suite only exercisesscore. A regression in the numeric coercion path would still pass here. Please add cases forpaymentsandnewest, ideally with string values like'10'vs'2'to lock in numeric ordering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/lib/sort.test.ts` around lines 93 - 100, The current sortAgentsWithTieBreaker test only covers the score tie-breaker, so it misses the updated payments and newest branches. Add focused cases in sort.test.ts that call sortAgentsWithTieBreaker with payments and newest, using string-like numeric values such as '10' and '2' to verify numeric coercion and ordering. Keep the assertions near the existing sortAgentsWithTieBreaker describe block so the coverage matches the changed comparison logic.frontend/components/ScoreBadge.tsx (1)
13-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the invalid-score log out of render.
This
console.errorruns during render, so repeated rerenders with the same bad prop will emit duplicate logs, and React StrictMode can amplify that in development. Move the log into the existinguseEffectkeyed onscoreso the fallback stays pure.Suggested change
export default function ScoreBadge({ score, showScore = true, size = 'md' }: Props) { const safeScore = Number.isFinite(score) ? score : 0; - if (!Number.isFinite(score)) { - console.error(JSON.stringify({ event: 'score_announcement_failed', error: 'Invalid score value' })); - } const [announcement, setAnnouncement] = useState(''); const previousScoreRef = useRef(safeScore); useEffect(() => { if (!Number.isFinite(score)) { + console.error(JSON.stringify({ event: 'score_announcement_failed', error: 'Invalid score value' })); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/ScoreBadge.tsx` around lines 13 - 16, Move the invalid-score logging out of the ScoreBadge render path: the console.error tied to the score fallback should not run while computing safeScore. Keep the fallback logic in ScoreBadge pure, and place the logging in the existing useEffect that already depends on score so it fires there instead of on every rerender. Use the ScoreBadge component and its useEffect as the reference points when updating this behavior.frontend/__tests__/WalletPickerModal.test.tsx (1)
1-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the wallet-kit mocks.
This file defines the same
jest.mock(...)factories twice. Keep a single block so the test's mock contract stays unambiguous and doesn't drift on later edits.Also applies to: 38-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/__tests__/WalletPickerModal.test.tsx` around lines 1 - 26, The wallet-kit mocks are duplicated in this test file, which makes the mock setup ambiguous and harder to maintain. Remove the repeated `jest.mock(...)` factory block in `WalletPickerModal.test.tsx` and keep only one consistent set of mocks for `StellarWalletsKit`, `FreighterModule`, `AlbedoModule`, `xBullModule`, `LobstrModule`, and `Networks` so the test contract is defined in a single place.frontend/__tests__/wallet.test.ts (1)
53-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the object-return branch too.
This refactor adds support for
fetchAddress()returning an object, but the updated suite only exercises the string path. Add one case that resolves{ address: 'GABC123' }and asserts the function still returns the normalized string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/__tests__/wallet.test.ts` around lines 53 - 62, The current wallet test only covers the string return path from connectWithWallet, so the new object-return branch in StellarWalletsKit.fetchAddress is untested. Add a test in wallet.test.ts that mocks fetchAddress to resolve an object with an address field, then verify connectWithWallet normalizes it to the same string result and still calls setWallet and fetchAddress as expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/agent.js`:
- Around line 258-259: Validate the env-derived retry settings in agent/agent.js
before they are used to filter candidates and retry tasks. In the code that
reads AGENT_MIN_SERVICE_REPUTATION and AGENT_MAX_SERVICE_RETRIES and in the
retry/candidate selection path that uses maxRetries with slice, reject
non-numeric or out-of-range values like 0 or NaN and fail fast with a clear
configuration error instead of silently producing no candidates. Update the
logic around the env parsing and the retry loop so the agent only proceeds when
the settings are valid.
- Around line 335-340: Move the payment header generation out of the main fetch
setup in runTask and into the retry-failure path so
encodePaymentSignatureHeader() is only called after a request failure is being
handled. Keep the existing failure handling flow in runTask and the fetch/catch
block, and ensure any header-generation errors are captured there so failure
logging, recordOutcome, failed-service tracking, and fallback candidate
selection still run.
---
Nitpick comments:
In `@frontend/__tests__/wallet.test.ts`:
- Around line 53-62: The current wallet test only covers the string return path
from connectWithWallet, so the new object-return branch in
StellarWalletsKit.fetchAddress is untested. Add a test in wallet.test.ts that
mocks fetchAddress to resolve an object with an address field, then verify
connectWithWallet normalizes it to the same string result and still calls
setWallet and fetchAddress as expected.
In `@frontend/__tests__/WalletPickerModal.test.tsx`:
- Around line 1-26: The wallet-kit mocks are duplicated in this test file, which
makes the mock setup ambiguous and harder to maintain. Remove the repeated
`jest.mock(...)` factory block in `WalletPickerModal.test.tsx` and keep only one
consistent set of mocks for `StellarWalletsKit`, `FreighterModule`,
`AlbedoModule`, `xBullModule`, `LobstrModule`, and `Networks` so the test
contract is defined in a single place.
In `@frontend/components/ScoreBadge.tsx`:
- Around line 13-16: Move the invalid-score logging out of the ScoreBadge render
path: the console.error tied to the score fallback should not run while
computing safeScore. Keep the fallback logic in ScoreBadge pure, and place the
logging in the existing useEffect that already depends on score so it fires
there instead of on every rerender. Use the ScoreBadge component and its
useEffect as the reference points when updating this behavior.
In `@frontend/lib/sort.test.ts`:
- Around line 93-100: The current sortAgentsWithTieBreaker test only covers the
score tie-breaker, so it misses the updated payments and newest branches. Add
focused cases in sort.test.ts that call sortAgentsWithTieBreaker with payments
and newest, using string-like numeric values such as '10' and '2' to verify
numeric coercion and ordering. Keep the assertions near the existing
sortAgentsWithTieBreaker describe block so the coverage matches the changed
comparison logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88fcd525-afb2-4289-a9a3-5a51ae171bfa
📒 Files selected for processing (16)
agent/agent.jsbackend/src/routes/agents.jsbackend/test/agents.test.jsfrontend/__tests__/AgentsPage.test.tsxfrontend/__tests__/RegistryPage.test.tsxfrontend/__tests__/WalletPickerModal.test.tsxfrontend/__tests__/wallet.test.tsfrontend/components/ScoreBadge.tsxfrontend/global.d.tsfrontend/lib/pagination.tsfrontend/lib/registry.test.tsfrontend/lib/sort.test.tsfrontend/lib/sort.tsfrontend/lib/types.tsfrontend/lib/wallet.tsfrontend/tsconfig.json
💤 Files with no reviewable changes (2)
- frontend/lib/registry.test.ts
- backend/test/agents.test.js
✅ Files skipped from review due to trivial changes (1)
- frontend/global.d.ts
Comment on lines
+258
to
+259
| const minReputation = parseInt(process.env.AGENT_MIN_SERVICE_REPUTATION ?? '0', 10); | ||
| const maxRetries = parseInt(process.env.AGENT_MAX_SERVICE_RETRIES ?? '3', 10); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate env-derived retry settings before filtering candidates.
Invalid values like AGENT_MAX_SERVICE_RETRIES=0 or abc make slice(0, maxRetries) produce no candidates, causing every task to fail without a clear config error.
Proposed fix
- const minReputation = parseInt(process.env.AGENT_MIN_SERVICE_REPUTATION ?? '0', 10);
- const maxRetries = parseInt(process.env.AGENT_MAX_SERVICE_RETRIES ?? '3', 10);
+ const parsedMinReputation = Number.parseInt(process.env.AGENT_MIN_SERVICE_REPUTATION ?? '0', 10);
+ const parsedMaxRetries = Number.parseInt(process.env.AGENT_MAX_SERVICE_RETRIES ?? '3', 10);
+ const minReputation = Number.isFinite(parsedMinReputation)
+ ? Math.max(0, parsedMinReputation)
+ : 0;
+ const maxRetries = Number.isFinite(parsedMaxRetries) && parsedMaxRetries > 0
+ ? parsedMaxRetries
+ : 3;Also applies to: 283-284
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/agent.js` around lines 258 - 259, Validate the env-derived retry
settings in agent/agent.js before they are used to filter candidates and retry
tasks. In the code that reads AGENT_MIN_SERVICE_REPUTATION and
AGENT_MAX_SERVICE_RETRIES and in the retry/candidate selection path that uses
maxRetries with slice, reject non-numeric or out-of-range values like 0 or NaN
and fail fast with a clear configuration error instead of silently producing no
candidates. Update the logic around the env parsing and the retry loop so the
agent only proceeds when the settings are valid.
naninu123
force-pushed
the
feat/dry-run-boost-scores
branch
from
af640b0 to
4cff9fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 50-51: The Stellar CLI install in the CI workflow is still
unpinned, so update the install step to use a known-good version instead of
tracking the latest crates.io release. Make the same install approach consistent
across the documentation by aligning CONTRIBUTING.md, README.md, and
contract/DEPLOY.md with the CI path, using the existing Stellar CLI install step
as the source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c27c53c-490c-4eac-b782-fc0f1d7b60f3
📒 Files selected for processing (1)
.github/workflows/ci.yml
Comment on lines
50
to
+51
| - name: Install Stellar CLI | ||
| run: cargo install --locked stellar-cli --features opt | ||
| run: cargo install --locked stellar-cli |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repo references =="
rg -n "stellar-cli|--features opt" .github/workflows/ci.yml CONTRIBUTING.md README.md contract/DEPLOY.md
echo
echo "== Latest published stellar-cli version =="
curl -fsSL https://crates.io/api/v1/crates/stellar-cli | jq -r '.crate.max_stable_version'Repository: Stellar-Ecosystem/lodestar
Length of output: 782
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant file excerpts =="
sed -n '35,70p' .github/workflows/ci.yml
echo
sed -n '1,40p' CONTRIBUTING.md
echo
sed -n '95,120p' README.md
echo
sed -n '1,40p' contract/DEPLOY.md
echo
echo "== All stellar-cli references =="
rg -n "stellar-cli|--features opt" .Repository: Stellar-Ecosystem/lodestar
Length of output: 4036
Pin stellar-cli and align the install command across docs.
cargo install --locked stellar-cli still tracks the latest crates.io release, so this CI job can start failing on an upstream stellar-cli bump. The docs also diverge (CONTRIBUTING.md uses --features opt, while README.md and contract/DEPLOY.md use install.sh). Pin a known-good version here and update the docs to use the same install path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 50 - 51, The Stellar CLI install in
the CI workflow is still unpinned, so update the install step to use a
known-good version instead of tracking the latest crates.io release. Make the
same install approach consistent across the documentation by aligning
CONTRIBUTING.md, README.md, and contract/DEPLOY.md with the CI path, using the
existing Stellar CLI install step as the source of truth.
ritik4ever
pushed a commit
that referenced
this pull request
Jun 29, 2026
…est expectations Restores structured logging (serviceName, priceUsdc, servicesFound, attempt fields), encodePaymentSignatureHeader, minReputation threshold, weighted fallback retry, and payment_failed error details that agent.test.js now expects (updated in PR #240).
naninu123
force-pushed
the
feat/dry-run-boost-scores
branch
from
b4f72cf to
b0396f0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #76
Summary
Adds
--dry-runmode tobackend/scripts/boost-scores.jsto prevent accidental on-chain transaction submission.What changed
backend/scripts/boost-scores.js— Parses--dry-runfromprocess.argv. When active, logs agent name, current score, target, needed payments, and total amount without callingrecordPaymentOnChain. Exportsboost()function with options object for testability. Top-level execution guarded behindimport.meta.urlcheck to avoid side effects when imported.backend/scripts/boost-scores.test.js(new) — 3 vitest tests covering:recordPaymentOnChainfor each needed paymentUsage
Dry-run output example
Test results
11/13 existing test suites pass (2 pre-existing failures unrelated to this change — JSX import issues in
agents.test.js).Summary by CodeRabbit