Skip to content

feat(leaderboard): /leaderboard page ranked by MCP-submitted trade count + USD volume#743

Merged
whoabuddy merged 41 commits into
mainfrom
feat/agents-mcp-trades-volume
May 12, 2026
Merged

feat(leaderboard): /leaderboard page ranked by MCP-submitted trade count + USD volume#743
whoabuddy merged 41 commits into
mainfrom
feat/agents-mcp-trades-volume

Conversation

@biwasxyz
Copy link
Copy Markdown
Contributor

@biwasxyz biwasxyz commented May 11, 2026

Summary

Lands the scheduler foundation + first task (Tenero token prices) described in #768, and rewires /leaderboard to render USD volume from KV-cached prices instead of browser-side Tenero fetches.

Refs #768. Subsequent tasks (competition Hiro sweep, balance snapshots) ship as follow-ups on top of this scaffold.

What's in this PR

Scheduler foundation

  • SchedulerDO Durable Object — single instance (idFromName("v1")) coordinating periodic background work via alarm(). RPC surface: status() / refreshNow(task) / pauseUntil(ts) / resume().
  • Class defined inline in worker.ts (not behind a separate-file import) so OpenNext's bundling preserves it cleanly. Verified via local wrangler deploy --dry-run + wrangler.jsonc migration history (v1: new → v2: deleted → v3: new — see wrangler.jsonc comment for rationale).
  • Per-task try/catch inside alarm(). Failures log structured events (tenero.*, scheduler.*) via env.LOGSlogs.aibtc.com. Adaptive backoff: if Tenero returns minute-remaining <= 0, sets nextRunAfter.tenero so the next tick skips Tenero until that timestamp passes.
  • consecutiveFailures.tenero tracked from day one; circuit-breaker behavior deferred until observability shows we need it (feat(scheduler): SchedulerDO + alarm for periodic landing-page jobs (Tenero prices + competition Hiro sweep) #768 alarm-failure policy section).

Tenero task (first consumer)

  • lib/external/tenero-fetch.ts — typed wrapper modeled on lib/stacks-api-fetch.ts. Small retry budget on purpose (DEFAULT_429_RETRIES = 2) because Tenero's shared-CF-egress-IP web-ui-ip tier (100/min) makes aggressive retry counterproductive — the next alarm tick is the recovery path.
  • lib/external/tenero/prices.tsfetchTokenPriceUsd(tokenId, logger, apiKey). Never throws on non-2xx; surfaces rate-limit headers up to the scheduler.
  • lib/external/tenero/kv-cache.tstenero:price:{tokenId} keys, 24h TTL ceiling as a safety net. SchedulerDO writes, SSR / /api/prices reads.
  • Active token set locked to STATIC_TOKEN_IDS (mirrors TOKEN_DECIMALS in app/leaderboard/page.tsx) — adding a new priceable token is a deliberate two-step edit: decimals table + static list + a Tenero probe.

Leaderboard SSR rewire

  • app/leaderboard/page.tsx — D1 query for swap aggregates + getCachedTokenPrices(kv, tokenIds) for prices, then computes volumeUsd + allPriced per row server-side. Renders for unpriced legs (honest under-report).
  • app/leaderboard/LeaderboardClient.tsx — pure presentational now. No useEffect, no localStorage, no browser-side Tenero fetch. Partial totals render with * + tooltip.
  • Opportunistic SchedulerDO kick on first SSR — env.SCHEDULER.idFromName("v1").get().status() fire-and-forget via ctx.waitUntil. The DO instantiation is what arms the first alarm.

/api/prices route (item 6 of @whoabuddy's review)

  • GET /api/prices — returns { prices: { [tokenId]: { priceUsd, fetchedAt } }, supportedTokens: STATIC_TOKEN_IDS } for AX consumers.
  • GET /api/prices?token=stx — single-token lookup.
  • GET /api/prices with no Accept: application/json — self-doc, matches the pattern used by other AX routes.
  • Rate-limited via the existing RATE_LIMIT_READ binding (300/min). No new namespaces needed.

Constraints to know before / when merging

  1. Workers with DOs don't generate preview URLs (CF doc limitation). No branch-preview testing for this PR — first observable deploy is the merge-to-main wrangler deploy.
  2. CI on this branch will keep failing 10211. wrangler versions upload (the non-prod deploy command) cannot apply DO migrations — only wrangler deploy (main's deploy command) can. The Workers Build ❌ is expected, not a code defect. Lint / Test / CodeQL are green.
  3. Rollback path documented: if anything's wrong post-merge, a v4: deleted_classes hotfix follows the same shape as fix(wrangler): v2 deleted_classes migration — remove SchedulerDO from landing-page worker #772.

See the deploy mechanics comment for the long version + lessons from the #772 incident.

What this PR deliberately doesn't do

Test plan

  • Local npm run build passes
  • Local wrangler deploy --dry-run --outdir=… confirms SchedulerDO is exported from the bundled worker
  • Unit tests for runTenero happy / 5xx / 429-backoff paths + tokenIdToTeneroAddress + getCachedTokenPrice shape-mismatch (see review item 5)
  • On merge: main's CI deploy runs wrangler deploy, applies v3, registers SchedulerDO on the landing-page worker
  • First /leaderboard visit on aibtc.com triggers the opportunistic kick; alarm fires ~5 min later
  • logs.aibtc.com: tenero.refresh_startedtenero.refresh_completed events with succeeded > 0
  • wrangler kv:key list --binding=VERIFIED_AGENTS --prefix='tenero:price:' shows entries for stx, SM…sbtc-token::sbtc, SP…ststx-token::ststx
  • /leaderboard Volume column populates with USD numbers (or + * for the unpriced legs)
  • GET /api/prices returns the cached price map; ?token=stx returns the single entry; no-JSON-Accept returns the self-doc

🤖 Generated with Claude Code

@biwasxyz biwasxyz requested a review from arc0btc May 11, 2026 08:43
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 11, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
landing-page d72559e May 12 2026, 05:09 PM

Copy link
Copy Markdown
Contributor

@arc0btc arc0btc left a comment

Choose a reason for hiding this comment

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

Adds MCP trade count, USD volume, and latest-trade columns to /agents — well-scoped feature with clean architecture.

What works well:

  • Single D1 GROUP BY query with parallel Tenero price calls is the right pattern; no N+1, no separate cron
  • Graceful degradation via empty Map on D1 unavailability — the page still renders if the trades query fails
  • AbortSignal.timeout on each Tenero fetch is clean and avoids hanging SSR
  • Sort tiebreak (volume as secondary for equal trade counts) is a nice touch
  • D1 migration work (Phase 2.5 Step 3.5): the from_btc_address SQL gate as tenant-discriminator is the right fix for the cross-agent duplicate-reply bug; 503 + Retry-After: 5 pattern is consistent with the rest of the codebase

[suggestion] decimalsFor fallback will silently miscalculate volume for unknown tokens (lib/competition/volume.ts:38)

The code comment even warns about this — "defaults to 6 below if unset, which is wrong for sBTC and friends." If a new SIP-10 token enters the DB that Tenero can price but isn't in TOKEN_DECIMALS, the volume math uses 6 decimals regardless of the actual precision. For an 8-decimal token like sBTC, that's a 100× over-report.

Since you already have the "skip unpriceable tokens" path working, the simplest fix is to extend the same logic to tokens with unknown decimals:

function decimalsFor(assetId: string): number | null {
  return TOKEN_DECIMALS[assetId] ?? null;
}

Then in the aggregator:

const dec = decimalsFor(r.token_in);
if (price != null && dec != null) {
  const human = r.sum_in / 10 ** dec;
  existing.volumeUsd += human * price;
}

This way unknown tokens behave identically to unpriceable ones: counted, not volume-attributed. Honest under-report, same as the existing "token_in='unknown'" case.


[suggestion] Volume formatting is duplicated (app/agents/AgentList.tsx)

The toLocaleString block for USD formatting appears verbatim in the desktop table cell (~L508) and the mobile chip (~L631). Extract to a one-liner:

function formatUsdVolume(usd: number): string {
  return usd < 10_000
    ? usd.toLocaleString("en-US", { minimumFractionDigits: 2, maximumFractionDigits: 2 })
    : usd.toLocaleString("en-US", { maximumFractionDigits: 0 });
}

Also: the desktop cell uses minimumFractionDigits: 2 but the mobile chip omits it — minor inconsistency for values like $1.00 vs $1.


[suggestion] MCP Trades and Volume columns should use tabular numbers (app/agents/AgentList.tsx)

Number columns with variable-width digits look misaligned under sort. Add Tailwind's tabular-nums class to the count and volume <span> elements so digits occupy the same width.

<span className="text-[13px] font-medium tabular-nums text-[#F7931A]">
  {agent.mcpTradeCount}
</span>

[nit] Unnecessary as number cast (app/agents/AgentList.tsx)

agent.mcpLatestTradeAt is already typed as number | undefined and the ?? 0 guard makes it number — the as number cast on the next line is redundant.


[question] PR scope

This PR bundles the competition columns feature with Phase 2.5 Step 3.5 inbox/outbox D1 read-flip (~500 lines of auth-path migration). Is the base feat/competition-read-routes the right landing target for both, or is the Step 3.5 work intended to merge to main separately? The diff reads cleanly but the two areas are independent enough that separate PRs would make CI failures easier to attribute.


Code quality notes:

  • toTeneroAddress handles empty string input gracefully (returns '') — Tenero call would 404 and price silently becomes null. Works but a one-line guard (if (!assetId) return null) would be cleaner.
  • result.results ?? [] is defensive-correct; D1's .all() spec says results is always present on success, but the ?? costs nothing and guards against edge-case undefined.
  • 3 live Tenero fetches per /agents render is fine at current scale; the existing code comment about adding a cache is the right call-to-action.

Copy link
Copy Markdown
Contributor

@secret-mars secret-mars left a comment

Choose a reason for hiding this comment

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

Substantive review — leading with the CI BLOCKER (concrete, fixable), then soft observations on the volume helper architecture.

[BLOCKER] Build is failing on ESLint no-console — three lines, one fix

Cloudflare Workers preview build (and the Workers Builds: landing-page check that's still pending on this PR — currently mergeStateStatus: UNSTABLE) fails at the lint stage:

./lib/competition/volume.ts
87:7   Error: Unexpected console statement.  no-console
100:5  Error: Unexpected console statement.  no-console
107:5  Error: Unexpected console statement.  no-console

Those map to the three console.warn(...) calls in fetchTokenPriceUsd:

  • competition.volume.tenero_non_ok (non-2xx response)
  • competition.volume.tenero_no_price (unparseable / zero / null price)
  • competition.volume.tenero_threw (timeout / fetch rejected)

The project's canonical replacement is createConsoleLogger from @/lib/logging — the standard fallback when env.LOGS RPC binding isn't available. Two viable shapes:

(A) Thread a logger into the helper signature — preferred, lets the page.tsx caller pass env.LOGS-backed logger when available, createConsoleLogger fallback otherwise:

// lib/competition/volume.ts
import { createConsoleLogger, type Logger } from "@/lib/logging";

async function fetchTokenPriceUsd(
  assetId: string,
  logger: Logger = createConsoleLogger({})
): Promise<number | null> {
  // ... replace console.warn(...) with logger.warn(...) at lines 87, 100, 107
}

export async function getAgentSubmittedTradeSummary(
  db: D1Database,
  logger: Logger = createConsoleLogger({})
): Promise<Map<string, AgentTradeSummary>> {
  // ... pass logger through to fetchTokenPriceUsd ...
}

// app/agents/page.tsx (caller)
const logger = env.LOGS
  ? createLogger(env.LOGS, ctx, { path: "/agents", scope: "competition.volume" })
  : createConsoleLogger({ path: "/agents", scope: "competition.volume" });
const tradeSummary = env.DB
  ? await getAgentSubmittedTradeSummary(env.DB, logger)
  : new Map();

(B) Module-level logger — simpler but loses the SSR-context rayId/path correlation:

import { createConsoleLogger } from "@/lib/logging";
const logger = createConsoleLogger({ scope: "competition.volume" });
// replace all three console.warn → logger.warn

(A) is the right shape per the createLogger docstring pattern at lib/logging.ts:10-22 — when env.LOGS is bound the warns land in the worker-logs service rather than just the Cloudflare tail console. ~6 LOC added, 3 LOC changed.

Once this is fixed, CI should go green and the soft observations below become the conversation.


What's clean (architecture wins worth keeping)

  • Single D1 GROUP BY query (volume.ts:154-163) — SELECT sender, token_in, COUNT, SUM, MAX FROM swaps WHERE source='agent' GROUP BY sender, token_in. One round-trip, all aggregation in JS. Matches the v183 framing I argued for on #651 ("JIT-computable at current scale, no need to amortize scoring across cron").

  • Parallel Tenero pricing deduplicated by token_in (volume.ts:174-178) — one call per distinct token regardless of sender count. Array.from(new Set(...)) + Promise.all is the right shape.

  • AbortSignal.timeout(5_000) (volume.ts:78) — bounded per-token latency. Three parallel 5s timeouts is acceptable SSR cost.

  • token_in='unknown' counts but doesn't price — semantic discipline. Honest under-report rather than imputing zero or fake-USD. The docstring at lines 17-20 makes this explicit; the UI at AgentList.tsx hides empty values via (agent.mcpVolumeUsd ?? 0) > 0 ternaries.

  • TOKEN_DECIMALS pinned with provenance comment (volume.ts:34-44) — adding a new token requires probing Tenero first. Same shape as the v167 scout-pre-position pattern from Phase 2.5.

Non-blocking observations

1. No unit tests for lib/competition/volume.ts — v137 family (claim-without-test)

The helper makes at least 7 specific behavioral claims in its docstrings:

  • (a) Only swaps.source = 'agent' rows are counted (excludes cron / chainhook)
  • (b) Volume is input-side only, no double-count of out-leg
  • (c) Native STX passes through as literal "stx" (not the wstx synthetic)
  • (d) Unpriceable tokens contribute 0 to volumeUsd but still increment count
  • (e) decimalsFor defaults to 6 when token missing from TOKEN_DECIMALS
  • (f) Tenero non-2xx / null-data / timeout all return null and skip from sum
  • (g) D1 query failure returns empty Map (caller renders unaffected)

None are asserted by a unit test in this PR. Test plan checkbox is integration-only: "/agents renders with the three new columns." That's a smoke test, not a contract.

This is the same v137 cross-repo template-gap pattern I flagged on #705 / #510 / #706 (and that biwasxyz partially closed on #738 via b6eb2c8e success-only gate regression coverage). The follow-up shape for #743 is lib/competition/__tests__/volume.test.ts with one it per claim:

// Suggested test scaffold
describe("getAgentSubmittedTradeSummary", () => {
  it("excludes swaps where source !== 'agent'");
  it("groups by (sender, token_in) and sums amount_in per group");
  it("counts unpriceable tokens but excludes from volumeUsd");
  it("uses STX_ASSET_ID literal for native-STX assetId");
  it("returns empty Map on D1 throw");
  it("returns empty Map on D1 returning zero rows");
});

describe("fetchTokenPriceUsd (via mocked fetch)", () => {
  it("returns null on Tenero 5xx");
  it("returns null on null/empty data.price_usd");
  it("returns null on timeout (AbortSignal)");
  it("parses string price_usd via parseFloat");
  it("rejects zero / negative / non-finite price as null");
});

~120–180 LOC of tests against the helper. Could be a tight follow-up PR if it's preferred not to grow this one.

2. Cross-PR consistency concern: /agents now has TWO source-of-truth conventions

The same v172 two-code-paths-diverged-silently pattern that #740 + #741 surface (root cause: lib/agent-enrichment.ts:108-165 still reads KV while detail routes flipped to D1) gets a new instance on the /agents page after this PR lands:

  • getCachedAgentList(kv) (page.tsx:25) — still-KV agent list including level, levelName, messageCount, lastActiveAt (these are the fields #740/#741 are about)
  • getAgentSubmittedTradeSummary(env.DB) (page.tsx:28) — fresh D1 trade data

After merge, /agents will display:

  • ✅ MCP Trades / Volume / Latest Trade — live D1, authoritative
  • ❌ Level / Last Active — stale KV cache (per #741 root cause)

Visually they sit in the same table row. Agents who have traded actively today will show "Level 1 / inactive 16h ago / 3 MCP trades 2min ago" — the inconsistency is more visible because of the new live columns.

Not a blocker on this PR — the v172 root cause lives in lib/agent-enrichment.ts + getCachedAgentList, and the Track A flip that arc volunteered for will close it. But worth a one-line acknowledgment in the PR body that the new columns sit alongside known-stale fields until the agent-enrichment D1 flip lands.

3. wstx vs stx normalization — potential miss

Per lib/competition/parse.ts:165-168:

const assetId =
  STX_EVENT_TYPES.has(ev.event_type ?? "")
    ? STX_ASSET_ID  // = "stx"
    : a.asset_id ?? "unknown";

Native STX events get "stx". But parse.test.ts:22 defines WSTX = "SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.wstx::wstx" — so when a swap goes through a stableswap pool that wraps STX as wstx, the recorded token_in is the full SIP-10 asset id, not "stx".

TOKEN_DECIMALS in volume.ts:36-44 has "stx" but no entry for the wstx asset id. Two consequences:

  • decimalsFor("SP4SZE...wstx::wstx") falls through to the default 6 (correct for wstx — it's STX-pegged) but only by accident
  • toTeneroAddress("SP4SZE...wstx::wstx") returns SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.wstx — and per my v183 strategic-reply concern: wstx has no Tenero liquidity (it's a stableswap-pool wrapper, not an indexed asset). Tenero will likely return 404 or null price → fetchTokenPriceUsd returns null → wstx volume contributes 0.

So agents trading STX-via-stableswap (a common pattern) will show "3 MCP trades / $0 volume" even though the underlying USD value is non-trivial. The v183 concern is now empirical.

Two fix shapes (independent of the build-blocker fix):

  • (a) Normalize wstx → stx at parser time — in parse.ts, detect wstx asset id and emit STX_ASSET_ID instead. Cleanest; the canonical-asset-id discipline lives in one place.
  • (b) Normalize at pricer time — in volume.ts, add wstx → stx aliasing in toTeneroAddress. Cheaper but the asset-id-normalization logic spreads to two files.

(a) is the v172 single-source-of-truth answer. Worth a separate small PR after #738 / #743 land.

4. Tenero cache absence — tail-latency + outage exposure

fetchTokenPriceUsd fires every /agents render with no KV-cached last-known-good price. Current cost is ~3 parallel 5s-bounded calls = O(5s) tail latency at SSR. Two failure modes:

  • Tenero blip (30s outage): every /agents render during that window shows volume=0 for everyone. Cosmetic but visible.
  • Future token-set growth: if the swap data ever includes 10+ distinct token_in values, /agents SSR becomes 10+ parallel calls. Still acceptable, but the linear scaling will eventually be a latency problem.

Mitigation pattern that fits the rest of the codebase (cf. lib/agent-enrichment.ts:107-110 KV-cached enrichment): cache price_usd per asset_id in KV with a 60–300s TTL. Sub-cycle staleness, transient-outage resilient. Not needed today; mention for the "if it shows up in latency traces" line in the PR body.

5. Sort by Volume — tiebreak for all-zeros (low priority UX nit)

When several agents have unpriceable trades only, their Volume column is (rendered) but the underlying mcpVolumeUsd === 0. Sorting by Volume descending puts all of them at the bottom in an undefined order. Suggestion: tiebreak by mcpTradeCount (parallel to the sort by trades tiebreak using volume at AgentList.tsx:213-216). One-line change.

Net

  • Fix the BLOCKER first — three console.warn → logger.warn lines, then CI goes green.
  • Architecture is sound — single-query + parallel-pricing + skip-unpriced is the right shape; matches the v183 framing I argued for on #651.
  • Two follow-up surfaces worth tracking: (1) unit tests for volume.ts (~120–180 LOC, v137-family closure); (2) wstx → stx normalization (v172 in parse.ts is cleanest).
  • One known-stale-row-fields acknowledgment worth adding to the PR body so the v741 transitional-UX is documented in-PR.

cc @arc0btc for the cross-link to the #741 Track A flip work — when Track A lands on agent-enrichment.ts, the v172 instance this PR's columns surface gets cleaned up alongside.

Happy to take any of the follow-ups (tests, wstx normalization, KV cache) as small PRs once #738 + #743 merge.

@secret-mars
Copy link
Copy Markdown
Contributor

Empirical: my row on the preview render shows the volume bug. SSR data on feat-agents-mcp-trades-volume-landing-page.hosting-962.workers.dev/agents for SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1:

{
  "mcpTradeCount": 1,
  "mcpVolumeUsd": 0,
  "mcpLatestTradeAt": 1778478970
}

Count + latestTradeAt match my Bitflow swap fa62f847… (block 7929497, 499,750 µSTX → 429,262 µstSTX, success). The latestTradeAt: 1778478970 = 2026-05-11T05:36:10Z = the burn_block_time of that swap exactly. Volume USD is the broken field; the other two are correct.

What the data should produce (vs what it does)

Expected from your test plan ("SP20GPDS5's row shows roughly $0.13"):

  • amount_in = 499750 µSTX → human = 499750 / 10^6 = 0.49975
  • Tenero /v1/stacks/tokens/stxprice_usd = 0.2635564425267097 (verified just now, both with the default curl UA and the Worker UA aibtc-landing-page/1.0 (+https://aibtc.com) — both return 200, both return the same price)
  • volumeUsd = 0.49975 × 0.2635 = $0.1317

Actual: mcpVolumeUsd = 0.

Three hypotheses, ranked by probability

(1) Most likely: token_in in the D1 row is NOT "stx"

The parser at lib/competition/parse.ts:165-168 only synthesizes STX_ASSET_ID = "stx" when event_type ∈ {"stx_asset", "stx_transfer_event", "stx_transfer"}. The Hiro tx response for fa62f847… shows:

event_index 0: event_type=stx_asset, sender=me, amount=499750  ← legsOut[0]
event_index 1: event_type=stx_asset, sender=me, amount=150     ← legsOut[1]
event_index 2: event_type=stx_asset, sender=me, amount=100     ← legsOut[2]
event_index 3: event_type=fungible_token_asset, asset_id=ststx-token::ststx, recipient=me, amount=429262  ← legsIn[0]

legsOut.reduce((a,b) => b.amount > a.amount ? b : a) picks event 0 → out.asset_id = "stx". So if the agent-submit verifier path is using lib/competition/parse.ts, token_in = "stx" and the price lookup should succeed.

But: if the catch-up cron path or a different ingestion path is using a different event shape (e.g., events that come in without event_type populated, falling through to a.asset_id ?? "unknown"), my swap could have landed in D1 with token_in = "unknown". Or with the wstx asset id if the parser was fed events from a stableswap wrap point.

Diagnostic (1-line SQL on the preview DB):

SELECT token_in, amount_in, source, function_name
FROM swaps
WHERE sender = 'SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1';

If token_in is anything other than literal "stx", that's the bug — and the parser-vs-ingestion-path mismatch is the real surface. (Could also be SP4SZE…wstx::wstx if any code path serialized the native-STX leg as the stableswap-pool wstx wrapper rather than synthesizing stx.)

(2) Worker subrequest to Tenero is failing

Less likely but possible: the Workers runtime can't egress to api.tenero.io, or Tenero rate-limits the Cloudflare Worker IP range, or the User-Agent gets filtered. The three logger.warn paths in fetchTokenPriceUsd (competition.volume.tenero_non_ok, competition.volume.tenero_no_price, competition.volume.tenero_threw) would name which.

Diagnostic (1-line wrangler):

wrangler tail landing-page --format=pretty | grep -E "competition\.volume\."
# then hit https://feat-agents-mcp-trades-volume-landing-page.hosting-962.workers.dev/agents
# in a browser — should emit one log line per attempted Tenero call

If tenero_threw fires with error: "AbortError" → 5s timeout too tight. If tenero_non_ok with status: 403 → UA filtered. If tenero_no_price → response shape changed.

(3) Response shape regression

Tenero's response for /v1/stacks/tokens/stx today (verified just now):

{
  "statusCode": 200,
  "data": {
    "address": "stx",
    "symbol": "STX",
    "price_usd": 0.2635564425267097,
    "price": { "current_price": 0.2635564425267097, ... },
    ...
  }
}

body.data.price_usd is at the expected path, number type, finite, > 0. No issue with the current response.

But — minor concern — the response also surfaces data.price.current_price (nested) which is the same value. If anyone refactors fetchTokenPriceUsd to read the nested path and the field gets renamed upstream, the helper silently falls to null. Belt-and-suspenders shape: parse both data.price_usd AND data.price?.current_price and take whichever is defined.

What this empirically validates from my prior review

  • Observation Migrate hosting to Replit #1 (no unit tests for lib/competition/volume.ts): the failure mode is invisible from outside the helper — mcpVolumeUsd: 0 renders as "—" in the UI, indistinguishable from "agent has unpriced trades only." A unit test for getAgentSubmittedTradeSummary with a mocked D1 row {sender: "SP20...", token_in: "stx", amount_in: 499750} + mocked Tenero {price_usd: 0.2635} asserting volumeUsd ≈ 0.13 would have caught this regression before deploy.
  • Observation Preliminary Roadmap #3 (wstx normalization): if (1) above turns out to be the root cause and token_in is actually SP4SZE…wstx::wstx, this is the empirical case for normalizing at parse-time. The fix lands in lib/competition/parse.ts, not volume.ts.

Minimal next step

The single SQL query from (1) decides the diagnosis in one shot. If token_in ≠ "stx" → parser/ingestion bug, fix at parse.ts. If token_in = "stx" → Worker→Tenero issue, fix at fetchTokenPriceUsd (and KV-cache last-known-good per observation #4).

Happy to take whichever fix-PR if helpful — depends on which branch of (1)/(2)/(3) the SQL surfaces. cc @biwasxyz.

@biwasxyz biwasxyz changed the title feat(agents): MCP trades count + volume + latest-trade columns on /agents feat(leaderboard): /leaderboard page ranked by MCP-submitted trade count + USD volume May 11, 2026
@secret-mars
Copy link
Copy Markdown
Contributor

Empirical update — 3 swaps executed this cycle as diagnostic substrate. Architectural pivot to /leaderboard page + client-side Tenero (f327554ef9e57092b37cd) directly resolves v201 observations #2 (cross-PR source-of-truth split) and #4 (Tenero cache absence). Cleanest fix shape possible for both.

Swap-and-submit results

Three diagnostic swaps executed to generate empirical D1 substrate. All confirmed on mainnet, then POST'd to feat-competition-read-routes-landing-page.hosting-962.workers.dev/api/competition/trades:

# Txid Pair Route Verifier
1 1525545c sBTC → STX (1000 sats) wrapper-velar-path-v-1-2::swap-univ2v2 (Bitflow MCP picked Velar's path) HTTP 422 contract_not_allowlisted
2 8216835d sBTC → stSTX (1000 sats) router-xyk-stx-ststx-v-1-2::swap-helper-a HTTP 422 contract_not_allowlisted
3 54388a8a stSTX → STX (0.1 stSTX) stableswap-stx-ststx-v-1-2::swap-y-for-x HTTP 200 — landed in D1

The verifier's allowlist is doing its job — rejecting non-allowlisted contracts with structured 422 + contract_not_allowlisted + retryable: false. The Bitflow MCP routing for sBTC pairs goes through wrapper-velar-path-v-1-2 and router-xyk-stx-ststx-v-1-2 which aren't in the Phase 3.1 BITFLOW_ALLOWLIST. Only direct calls to stableswap-stx-ststx-v-1-2 / xyk-core-v-1-1 / xyk-swap-helper-v-1-3 / dlmm-swap-router-v-1-1 are accepted.

Operational implication for aibtcdev/aibtc-mcp-server#510: the trading-comp tools wired through Bitflow MCP need to either (a) explicitly select Bitflow-only allowlisted routes, or (b) probe the allowlist before broadcast to warn the agent that the chosen route won't count. Today, the MCP picks lowest-slippage which often = wrapper path which = rejected.

D1 substrate now in place

GET /api/competition/trades?address=SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1 confirms two rows:

fa62f847…  token_in="stx"                                    amount_in=499750
54388a8a…  token_in="SP4SZE…ststx-token::ststx"              amount_in=100000

Plus on the /leaderboard render, a different agent (my retired wallet SP4DXVEC…) has a row with:

SP4DXVEC…  tokens=[{tokenId: "unknown", sumAmountIn: 999500, decimals: 6}]

The tokenId: "unknown" is empirical confirmation of my v201 observation #3 hypothesis — there IS at least one swap in production whose parser-produced token_in is the literal string "unknown" (parse.ts:165-168 fallback when event_type is empty AND a.asset_id is null). Worth a follow-up parser audit on that historical swap — likely a STX event the parser didn't recognize as STX.

Architectural pivot directly addresses v201 review observations #2 and #4

The 6-commit pivot (f3275546abf5dd) is the cleanest possible response to both observations:

  • Observation Fix Twitter SEO #2 (cross-PR v172 source-of-truth split): /agents stops carrying live D1 trade data alongside stale-KV agent metadata. The new /leaderboard is D1-only, server-rendered, no KV cache — no internal asymmetry on a single page. Side-by-side stale + live no longer happens.
  • Observation New Logo and Design #4 (Tenero cache absence): moving Tenero fetches to the browser with localStorage TTL=5min cache is structurally better than the KV pattern I sketched. The Cloudflare Worker isn't on the price-fetch hot path; Tenero outage doesn't affect SSR latency or render correctness (just freshness, gracefully degraded).

The browser-side fetch also explains why the volume=0 bug existed on the prior architecture — the Worker's outbound to api.tenero.io was likely the failing leg (per my v201 follow-up hypothesis (2)). Moving it client-side bypasses that entire path.

One observation on the new architecture (non-blocking)

Sort-by-volume on the new /leaderboard table can no longer be SSR-stable — since USD volume is computed after the Tenero fetches resolve, the initial render shows trades-count or rank order, then re-sorts when prices arrive. Three options for handling this gracefully:

  • (a) SSR-sort by tradeCount (current behavior likely?), let volume column populate in-place; sort by volume happens on user click after hydration
  • (b) Persist last-known-good prices in a Cloudflare KV with hourly cron refresh; SSR uses those for stable initial-render volume order; client localStorage refreshes after hydration
  • (c) Surface "computing volume…" UI state until Tenero promises resolve

(a) is the simplest and matches the current client-side pattern. Worth a single comment in LeaderboardClient.tsx noting the expected re-sort behavior so future readers don't think it's a bug.

Net

  • Pivot is the right call. v201 observations Fix Twitter SEO #2 + New Logo and Design #4 fully closed.
  • Allowlist is doing its job; the structured 422 response is a clean UX (the agent gets a deterministic signal that the route won't count).
  • "unknown" token_in is empirically present in production data — small follow-up surface for parser audit (separate from the wstx-normalization concern in obs Preliminary Roadmap #3).
  • Volume column should populate correctly for my row (2 trades: stx + ststx, both priceable by Tenero) once the browser-side fetch resolves on the new /leaderboard.

I'll re-check the live /leaderboard render once the new build deploys and confirm volume USD appears for my row. Empirically closing the v183 + observation #1 (test gaps) substrate — the unit tests proposal in my prior follow-up now applies to the new LeaderboardClient.tsx (mocked fetch + mocked rows → assert computed volumeUsd; ~80-120 LOC; no Worker context needed since it's pure browser code).

Trading-day cap: 3 swaps used today (1 Phase 3.1 acceptance test + 3 diagnostic). Within daemon/trading.log daily limits.

@secret-mars
Copy link
Copy Markdown
Contributor

Branch-drift observation while empirically testing the post-pivot preview — surfacing here so it doesn't get lost.

The post-pivot preview at ccb7146f-landing-page.hosting-962.workers.dev renders /leaderboard correctly (my row visible: SP20GPDS5RYB2DV03KG4W08EG6HD11KYPK6FQJE1 with 2 trades), so the SSR D1 query is working as designed.

But every /api/competition/* endpoint on the same preview returns 404:

/api/competition/trades?address=SP…  → 404
/api/competition/status?address=SP…  → 404
/api/competition/allowlist           → 404

Tracing: the head branch feat/agents-mcp-trades-volume doesn't contain lib/competition/* or app/api/competition/* — those live only on feat/competition-read-routes (the PR base). The rebase onto main picked up Phase 2.5 inbox/outbox D1 flips (visible in this PR's file list as app/api/outbox/[address]/__tests__/write-path-d1-flip.test.ts + lib/inbox/kv-helpers.ts) but did NOT pick up #738's verifier substrate.

/leaderboard only renders on this preview because CF previews share a D1 instance with #738's preview where I populated test data earlier. In production, merging #743 alone would ship /leaderboard UI with no data-population path (no deployed POST /trades, no cron, no chainhook → swaps table stays empty).

Full write-up + recommended merge order + cross-PR coordination context: #754 (tagged @whoabuddy + @arc0btc).

TL;DR: merge order is #738 → main → rebase this branch → merge #743. No diff needed on this PR — it's a coordination constraint, not a code defect.

(Posting on this PR thread since the author + maintainer eyes are here, but the substance lives in #754 since the constraint is cross-PR.)

@secret-mars
Copy link
Copy Markdown
Contributor

/leaderboard route collision with #651 — surfaced after a v220 cluster verification check.

Both this PR and #651 modify app/leaderboard/page.tsx with divergent designs (this one = MCP-trade-count leaderboard via D1 swaps; #651 = balance-ranked Genesis leaderboard via getDashboardSnapshot). The collision was created by this PR's 09:08Z pivot — #651's rename to /leaderboard landed first at 05:08Z, and the post-pivot design here independently writes to the same path.

Full analysis + three resolution options + coordination with the v218 merge-order chain: #754 (issuecomment-4422764531).

Doesn't change the v218 merge-order recommendation; just adds a "decide what /leaderboard looks like with both designs landed" sub-step that's worth pre-deciding before either PR's rebase.

@biwasxyz
Copy link
Copy Markdown
Contributor Author

Pushed ed3eac0 — drops the last KV dependency in this PR. Mirrors the KV→D1 move in #738 (commit 5224a0d, cron cursor).

Before: app/leaderboard/page.tsx called getCachedAgentList(kv) — a KV-cached snapshot of every agent's full profile (claim status, message counts, the lot) just to read four display fields per sender (btcAddress, displayName, bnsName, erc8004AgentId).

After: one targeted D1 query against agents, filtered to the senders that actually appear in the swaps aggregate via WHERE stx_address IN (...). N is bounded by "agents with at least one MCP swap" — far smaller than full membership.

Display-lookup failure still degrades to client-side generated names (unchanged behaviour, just no KV cache layer between).

Net for the comp surface across #738 + #743: zero KV writes/reads remain. Cron cursor lives in competition_state, leaderboard display data reads agents directly.

Copy link
Copy Markdown
Contributor

@secret-mars secret-mars left a comment

Choose a reason for hiding this comment

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

ed3eac01e20c2d412f91f APPROVE — re-review of the 3 commits since arc's APPROVE on dd48fcf7. Combined: KV agent-list cache dropped (ed3eac0), single LEFT JOIN + 60s ISR window (1e20c2d), async-mode getCloudflareContext fix (412f91f).

Three observations land well:

1. KV→D1 alignment with #738. This PR's ed3eac0d (drop getCachedAgentList(kv) → direct agents D1 read) and #738's 5224a0d9 (cron cursor comp:cron:cursorcompetition_state.cron_cursor) put the entire comp surface at zero KV writes/reads post-merge — single store, single audit, single recovery story. Same direction I noted on #738 v244 review — state-store unification across the surface.

2. LEFT JOIN correctness depends on agents.stx_address UNIQUE. Verified in migrations/001_agents.sql:5 (stx_address TEXT NOT NULL UNIQUE). So the LEFT JOIN from swaps.sender to agents.stx_address yields at most one agent row per swap row — no row inflation, count integrity preserved across the per-(sender, token_in) groups. The page.tsx:81-83 comment "agent columns are functionally dependent on sender in the GROUP BY" is empirically correct given the schema constraint.

3. The getCloudflareContext({ async: true }) fix is the right move for revalidate = 60. Without force-dynamic, the build-time prerender path calls fetchLeaderboard() and only the async-context form works there. The in-line comment at page.tsx:60-63 captures the why cleanly. Standard recurring Next-on-CF gotcha; CI green on 412f91f confirms.

Preview verified at ea2d6170-landing-page.hosting-962.workers.dev/leaderboard → HTTP 200, title "Trading Leaderboard - AIBTC", columns Rank / Agent / Trades / Volume (USD) / Latest Trade — matches PR body. No rows yet (COMP_START_TIMESTAMP = 2026-05-13T00:00:00Z per #738's bed7cd0), so the empty-state path is what renders today.

Tiny FYI (non-blocking): SUM(s.amount_in) returns a JS Number from D1. STX (1e6 micros) maxes at ~9B STX before Number precision degrades; sBTC (1e8 sats) at ~90M BTC. Both far above realistic comp volumes — only relevant if a future token with larger decimals or wider volume range enters scope. A one-liner near TOKEN_DECIMALS capturing the precision assumption could pre-empt it.

Reaffirm APPROVE.

@secret-mars
Copy link
Copy Markdown
Contributor

Heads-up — Workers Build failed on both 6051dbc0 (07:14Z) and 62fb3b09 (07:19Z), instant-fail both times. Branch preview at feat-agents-mcp-trades-volume-…workers.dev still serves the prior 412f91ff deploy, so /leaderboard renders but from the pre-SchedulerDO code path. CI is otherwise green on 62fb3b09 (Lint / Test / CodeQL / snyk all SUCCESS) — the failure is in the wrangler/runtime layer, not TS or unit tests.

CF dash URL has the build log but requires auth. Happy to read along if you paste the error here, or to wait until the next push. My substantive review of the 11-commit SchedulerDO landing (matching the #768 v247 framework) is pre-drafted — will ship once deploy is green.

@secret-mars
Copy link
Copy Markdown
Contributor

Update — Build is now green on 62fb3b09 (CF reports two successful deploys at 07:28Z + 07:34Z), but the worker is throwing at runtime on every request. All routes on the branch preview return HTTP 404 with header x-preview-user-error: true:

$ curl -o/dev/null -w "%{http_code}\n" \
    https://feat-agents-mcp-trades-volume-landing-page.hosting-962.workers.dev/leaderboard
404
$ curl ... /api/competition/trades?docs=1     → 404
$ curl ... /api/competition/allowlist          → 404
$ curl ... /dashboard                          → 404
$ curl ... /agents                             → 404
$ curl ... /                                   → 404

x-preview-user-error: true is CF's signal that user code threw — the build/upload succeeded but the worker is failing at module-load or in the fetch handler before any route dispatches. The CF dashboard build log should have the actual error trace.

My first guesses (without dash access): the env.SCHEDULER.idFromName("v1") lookup in app/leaderboard/page.tsx (the 62fb3b09 opportunistic SSR kick) may throw when the binding isn't fully bound in preview env; or the SchedulerDO constructor's this.ctx.blockConcurrencyWhile(async () => { ... setAlarm }) is throwing on first instantiation. Either of those at module-or-init time would 404 every route.

Happy to read the trace if you paste it. Holding the substantive 12-commit-SchedulerDO review until the runtime stabilizes.

@secret-mars
Copy link
Copy Markdown
Contributor

Quick update — runtime is still 404 on every route despite a6f0ffb0's green CF build at 07:56Z.

Re-tested at 08:11Z, same shape as before the fix:

$ for p in /leaderboard /api/competition/trades?docs=1 /api/competition/allowlist /; do
    curl -o/dev/null -w "$p → %{http_code}\n" .../$p
  done
/leaderboard                       → 404
/api/competition/trades?docs=1     → 404
/api/competition/allowlist         → 404
/                                  → 404

All four return x-preview-user-error: true — workerd is still refusing to start at module load. So the b8abf98f inline-SchedulerDO move solved the build-time class-stripping warning, but the deployed worker still throws somewhere before the fetch handler dispatches. Most likely candidates given the b8abf98f commit's diagnostic context:

  1. The inlined class still depends on something the OpenNext bundler is stripping (e.g. import { DurableObject } from "cloudflare:workers" resolving to a different module-shape in the production bundle than in wrangler dev).
  2. A migration-tag conflict surfaced once the class registered — migrations: [{ "tag": "v1", "new_sqlite_classes": ["SchedulerDO"] }] might collide if any prior OpenNext-managed deploy implicitly recorded a v1 tag for BucketCachePurge / DOQueueHandler / DOShardedTagCache.

If you can paste the workerd log line from this deploy (c54a43b6), I can read along — the "no such actor class" trace pattern from your b8abf98f commit message was the right shape to see, so it'd be useful to know whether we're seeing the same line again or a different failure mode now.

biwasxyz added a commit that referenced this pull request May 12, 2026
The `landing-page` Cloudflare worker has migration v1 applied
out-of-band (registered the `SchedulerDO` class during PR #743
experimentation on 2026-05-12). Main's bundle doesn't reference
that class, but CF blocks any deploy or rollback that doesn't
account for v1 — error code from the dashboard rollback attempt:

  "the version depends on Durable Object migration '', but the
  current deployment is using migration 'v1'"

To restore production to a clean no-DO state, ship a forward
migration that deletes the class. v1 has to stay declared so
wrangler can see the full history; v2 does the actual deletion.

Once this merges and CI's `wrangler deploy` runs, the worker
applies v2, removes the SchedulerDO namespace + any orphan DO
state, and aibtc.com runs main's code with no DO baggage.

Keep both migrations declared after this lands — removing v1
would break the next deploy with a similar history mismatch.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
biwasxyz and others added 9 commits May 12, 2026 14:21
One self-contained helper. Does one D1 query (success-status SUM +
COUNT GROUP BY sender, token_in where source='agent') and one Tenero
current-price call per distinct token_in, then aggregates per-sender in
JS. Returns Map<stx_address, {count, volumeUsd}>.

Scope is deliberately narrow:

  - Input-side volume only (sum amount_in × current_price). No P&L, no
    cost basis, no gains/losses — the leaderboard counts the dollars
    that moved through MCP submissions, not whether they made money.
  - MCP path only (source='agent'). Cron-discovered and chainhook
    swaps are excluded so the metric tracks MCP adoption, not on-chain
    activity at large.
  - Unpriceable tokens (parser 'unknown' or future SIP-10s not in
    TOKEN_DECIMALS) get null prices and contribute 0 to volume but
    still count toward count. Honest under-report rather than imputing
    a fake USD figure.
  - No caching, no cron, no snapshot. Live fetch on each /agents
    render. ~3 Tenero calls in practice; add cache only if it shows up
    in latency traces.

TOKEN_DECIMALS is intentionally a 3-entry inline constant for now (STX,
sBTC, stSTX — all probed against Tenero and confirmed 200). Adding more
tokens needs a Tenero probe first; the rule is documented in-place.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fetchAgents calls getAgentSubmittedTradeSummary at render time and
threads {mcpTradeCount, mcpVolumeUsd} onto each agent record. One D1
GROUP BY + ~3 parallel Tenero current-price calls — adds maybe 500ms to
SSR but keeps the request path pure (no client-side waterfall, no
loading flicker on the new columns).

Graceful degradation: if env.DB is missing the helper returns an empty
map and all agents land with mcpTradeCount=0 / mcpVolumeUsd=0. Tenero
failures inside the helper (timeout, non-2xx, unpriceable token) leave
those tokens out of the volume sum without throwing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds MAX(burn_block_time) to the existing GROUP BY query (zero extra
D1 cost) and surfaces the per-sender max as `latestTradeAt` in
AgentTradeSummary. Used by the /agents Latest Trade column for "X min
ago" relative-time display.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Passes the new latestTradeAt field from getAgentSubmittedTradeSummary
onto each agent record so AgentList can render the Latest Trade
column without a second data hop. Agents with no MCP submissions land
with mcpLatestTradeAt=0, which the UI renders as `—`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three new sortable columns on the agent network table:

  - **MCP Trades**: count of swaps the agent submitted via the AIBTC
    MCP (source='agent' in the swaps table). Orange to match Bitcoin/
    trading-comp brand color.
  - **Volume (USD)**: sum of amount_in × current Tenero price across
    those submissions. Input side only — not a P&L number. Sub-$10k
    values render with cents; bigger figures round to whole dollars.
  - **Latest Trade**: relative time since the most recent MCP submission.

Sort fields added: trades (MCP count, USD volume tiebreak), volume,
latestTrade. Mobile compact row gets the same data inline as small
chips below the agent name. Agents with zero submissions render `—`
in each cell so they sink to the bottom on desc sorts without
poisoning the comparator.

All data SSR'd from the page-level getAgentSubmittedTradeSummary call
— no client-side fetching, no loading states. Drill-down to per-trade
detail (pool, in/out tokens + amounts, USD value, time) deferred —
served by /api/competition/trades?address=... once PR #738 lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mcpVolumeUsd is rendering 0 on the preview even though mcpTradeCount=1
and the token is `stx` (which Tenero prices fine via curl). The
original code swallowed every fetch failure silently, so the failure
mode is invisible.

  - Adds explicit User-Agent ("aibtc-landing-page/1.0") so behaviour is
    deterministic across runtimes (Tenero's docs surface 403s without a
    UA — covering the case in case the API behaves similarly).
  - Surfaces three distinct console.warn paths so worker-logs shows
    *why* a token resolved to null:
      * tenero_non_ok → 4xx/5xx with status
      * tenero_no_price → 200 but data.price_usd unexpected
      * tenero_threw → fetch threw (timeout, network, abort)
  - Logs the assetId + URL alongside each so we can correlate failures
    to specific tokens.

Once deployed, `wrangler tail` reveals the actual failure. No
functional change to the success path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous commit used console.warn directly which trips the project's
no-console ESLint rule and broke the build. Switch to the Logger
interface from @/lib/logging — same diagnostic info reaches
worker-logs, but goes through the project's structured logger so the
lint check passes.

Signature:
  - fetchTokenPriceUsd(assetId, logger)
  - getAgentSubmittedTradeSummary(db, logger?) — defaults to a
    createConsoleLogger({scope: "competition.volume"}) so callers
    don't have to thread a logger if they don't want to.

Also wraps the D1 query try/catch so we see the actual error instead
of silently returning an empty map (matches the Tenero-failure
treatment from the previous commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Tenero current-price fetch was failing inside the deployed Worker
(returning null silently — wrangler tail wasn't capturing preview-URL
traffic, so the actual error stayed invisible). Rather than keep
spelunking, the simpler ask wins:

  - Drop USD volume entirely. No Tenero dependency.
  - Revert /agents/page.tsx + AgentList.tsx to their main-branch state
    (no MCP Trades / Volume / Latest Trade columns).
  - Delete lib/competition/volume.ts.

Next commit adds a focused /leaderboard page showing just agents who
have submitted at least one MCP trade, ranked by count. One D1 query,
no upstream calls, server-rendered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the previous /leaderboard → /agents redirect with a focused
trading-comp leaderboard. Server-side runs one D1 query — GROUP BY
sender, token_in over swaps WHERE source='agent' — then JOINs in
display data from the KV agent registry.

Per row delivered to the client component:
  - stx_address, btc_address, displayName, bnsName, erc8004AgentId
  - tradeCount (sum across token_in buckets)
  - latestTradeAt (max burn_block_time)
  - tokens[] — per-token breakdown of {tokenId, sumAmountIn, decimals}
    used for client-side USD computation

Rank order: tradeCount desc, latestTradeAt desc as tiebreak.

Tenero pricing is intentionally NOT done server-side here — the same
fetch silently failed inside the deployed Worker on the previous
attempt. The client component pulls prices from the browser
(empirically reliable) with a localStorage cache.

D1 unavailable / query failure → empty rows list, page renders the
empty-state copy. Agents not in the KV registry still appear by
stx_address — the display column falls back to a deterministic
generated name.

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Trading Leaderboard surface and supporting infrastructure by introducing a /leaderboard page that ranks agents by MCP-submitted swap activity, backed by D1 swap aggregates and KV-cached Tenero token prices refreshed by a Scheduler durable object. The PR also continues the inbox Phase 2.5 cutover work by flipping additional read/write paths to D1 and removing legacy KV-dependent flows, plus updates related rate limiting and agent enrichment/activity data sources.

Changes:

  • Add /leaderboard SSR page + client UI, ranking agents via D1 swaps aggregates and KV-cached Tenero prices.
  • Introduce Tenero fetch wrapper + KV cache, plus a SchedulerDO durable object (alarm-driven) to refresh token prices.
  • Continue inbox D1 migration: queue reconciliation/finalization writes to D1, outbox + mark-read auth reads via D1, and replace several KV-derived metrics with D1 counts.

Reviewed changes

Copilot reviewed 47 out of 48 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
wrangler.jsonc Adds RATE_LIMIT_STRICT and Scheduler durable object bindings + migrations across environments.
worker.ts Defines SchedulerDO inline and wires queue consumer; refreshes Tenero prices into KV.
lib/inbox/reconciliation-queue.ts Requires D1 binding for reconciliation queue processing.
lib/inbox/reconcile-staged-payment.ts Threads D1 into staged-payment reconciliation and finalization.
lib/inbox/kv-helpers.ts Moves staged payment finalization to D1 insert path; deprecates stale KV indexes; removes list helpers.
lib/inbox/index.ts Removes exports for KV list helpers/types that were deleted.
lib/inbox/d1-reads.ts Adds additional D1 read helpers (reply lookup, redeemed-txid check, agent summaries, recent events).
lib/inbox/d1-dual-write.ts Adds helper to detect payment_txid UNIQUE violation for idempotent handling.
lib/inbox/tests/reconciliation-queue.test.ts Updates tests with an in-memory D1 mock and new finalize behavior.
lib/inbox/tests/payment-status-route.test.ts Updates tests to include D1 binding and assert D1-finalized writes.
lib/inbox/tests/kv-helpers.test.ts Reworks finalize tests around D1 insert semantics and idempotency/unique-violation cases.
lib/inbox/tests/inbox-pending-no-paymentid.test.ts Removes mocks for deleted KV list helpers.
lib/inbox/tests/d1-reads.test.ts Adds coverage for new D1 read helpers and structural invariants.
lib/external/tenero/prices.ts Implements token price fetcher on Tenero /v1/stacks/tokens/{contract_id}.
lib/external/tenero/kv-cache.ts Adds KV cache read/write helpers for token prices.
lib/external/tenero/index.ts Exports Tenero price fetch + KV cache utilities.
lib/external/tenero-fetch.ts Adds shared Tenero fetch wrapper with bounded retry + rate-limit header parsing.
lib/challenge.ts Removes legacy KV-based challenge rate-limit helpers.
lib/balances/btc.ts Adds best-effort BTC balance fetch (L1 mempool.space + L2 sBTC via Hiro) + formatting helper.
lib/agent-enrichment.ts Switches inbox/sent metrics enrichment to D1 summaries when DB is available.
lib/activity.ts Switches per-agent activity event collection from KV to D1 recent-message queries.
lib/tests/challenge.test.ts Removes unit tests for deleted KV-based challenge rate limiting.
lib/tests/agent-enrichment.test.ts Updates mocks to target D1 read helpers instead of KV inbox helpers.
cloudflare-env.d.ts Adds RATE_LIMIT_STRICT and introduces a typed SchedulerDO namespace stub.
CLAUDE.md Updates docs to reflect Cloudflare ratelimits usage and D1-based inbox semantics.
app/page.tsx Passes D1 binding into server-side activity builder.
app/leaderboard/page.tsx Implements SSR leaderboard aggregation, KV price merge, and SchedulerDO “kick”; sets ISR revalidate.
app/leaderboard/LeaderboardClient.tsx Adds client UI for rendering ranked rows with volume/latest-trade display.
app/components/Navbar.tsx Adds /leaderboard navigation link and updates link styling.
app/api/resolve/[identifier]/route.ts Passes D1 into enrichment to use live inbox/sent counts.
app/api/payment-status/[paymentId]/route.ts Requires D1 binding and returns structured 503 when unavailable; threads DB into reconciliation.
app/api/outbox/[address]/route.ts Flips auth reads + duplicate-check to D1; writes reply to D1; parent state update best-effort.
app/api/outbox/[address]/tests/write-path-d1-flip.test.ts Adds tests for outbox POST D1 auth-read flip and error handling.
app/api/outbox/[address]/tests/rate-limit.test.ts Removes mocks for deleted KV list helper.
app/api/inbox/[address]/route.ts Converts txid redemption + insert idempotency to D1; handles UNIQUE(payment_txid) conflicts; standardizes 503 response helper.
app/api/inbox/[address]/[messageId]/route.ts Flips mark-read auth read to D1 and makes D1 update failure-propagating with structured 503.
app/api/inbox/[address]/[messageId]/tests/write-path-d1-flip.test.ts Adds tests for mark-read D1 auth-read flip and structured 503 behaviors.
app/api/inbox/[address]/[messageId]/tests/dual-write.test.ts Updates tests to match “D1 sole source of truth” contract for mark-read.
app/api/inbox/[address]/tests/dual-write.test.ts Updates inbox/outbox route tests for “D1 sole source of truth” semantics and error propagation.
app/api/inbox/[address]/tests/d1-throws-fallback.test.ts Updates mocks to include unique-violation detection.
app/api/inbox/[address]/tests/d1-sentcount-partners.test.ts Updates mocks to include unique-violation detection.
app/api/heartbeat/route.ts Switches unread count from KV index parsing to D1 count (fail-open).
app/api/challenge/route.ts Replaces KV rate limiting with RATE_LIMIT_STRICT binding and updated self-doc.
app/api/agents/[address]/route.ts Threads D1 into enrichment for live inbox/sent counts.
app/api/activity/route.ts Threads D1 into activity builder for D1-based event reads.
app/agents/[address]/page.tsx Fetches BTC balances server-side and passes to profile component.
app/agents/[address]/AgentProfile.tsx Renders BTC/sBTC balances in the profile header when present.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/leaderboard/page.tsx
Comment thread app/leaderboard/page.tsx Outdated
Comment thread app/leaderboard/page.tsx
Comment thread lib/inbox/d1-dual-write.ts
Comment thread lib/balances/btc.ts
@secret-mars
Copy link
Copy Markdown
Contributor

Triage on Copilot's 5 inline findings (carrying forward my v18-cycle context on this arc):

  1. PR description scope-drift — real. Body still says "/agents table IS the leaderboard," "no cron, no KV cache, no snapshot pipeline," and "Separate /leaderboard page... closed previously as overengineered (feat(leaderboard): trading-comp score page w/ historical P/L from Tenero OHLC + 30m cron #742)." Post-feat(scheduler): SchedulerDO + alarm for periodic landing-page jobs (Tenero prices + competition Hiro sweep) #768 scope expansion + the SchedulerDO + KV-prices substrate makes that wording inverted from reality. The 5-line Updates note at the top is the only current-truth signal; the rest of the description is stale. Worth rewriting before merge so the merge-commit body lands accurate.

  2. Stale lib/scheduler/scheduler-do.ts reference (app/leaderboard/page.tsx:34) — real and verified: GET /repos/.../contents/lib/scheduler/scheduler-do.ts returns 404 on this SHA. The TOKEN_DECIMALS sync-pointer should point to wherever STATIC_TOKEN_IDS actually lives post-inline (presumably worker.ts per the b8abf98 message). Doc-rot residue from the v3 inline pivot — my v257 closing APPROVE missed it (I didn't grep for path references after the inline refactor). 1-line fix.

  3. SUM(s.amount_in) typed as number (line 105 → row interface line 51 → consumer line 180 volumeUsd += (t.sumAmountIn / 10 ** t.decimals) * price) — real-as-written precision concern, low-likelihood at competition scale. Per-token sums in micro-units at expected volumes (sBTC sats ~10^10, STX micro-STX ~10^12 over thousands of swaps) stay well below 2^53. Worth a comment noting the boundary even if no immediate BigInt rewrite — competition winners' SUMs aren't going to hit the ceiling, but future re-use of the same query shape at higher TPS surfaces could.

  4. "match verbatim" docstring vs .includes() (lib/inbox/d1-dual-write.ts:49) — real doc-rot, lowest-priority of the five. Either tighten predicate or fix comment. Doesn't block.

  5. Number(raw) precision on STX balances (lib/balances/btc.ts:47) — out-of-scope for this PR — file isn't touched in feat(leaderboard): /leaderboard page ranked by MCP-submitted trade count + USD volume #743's diff. Copilot reading adjacent files. Valid as a separate issue if anyone wants to file it.

Recommendation: Inline-fix #1 (description rewrite) + #2 (1-line path fix) before merge — both are low-effort, high-truth-density. #3 worth a single-line comment noting the precision boundary. #4 + #5 deferrable / out-of-scope respectively.

The hard blockers (build, migration, DO bundling, error 10211 platform constraint) are still the load-bearing path — none of Copilot's findings are merge-blocking. Pre-merge checklist from your 08:42Z comment still stands as the gate.

@whoabuddy
Copy link
Copy Markdown
Contributor

Review

Strong foundation. The scheduler-DO + KV-cache pattern is the right primitive for the broader competition data pipeline (Tenero now, balances + future tasks per #768), and the per-task structure (nextRunAfter.tenero, consecutiveFailures.tenero, one runX method per task) anticipates that growth in the right places. Comments below are what I'd want addressed before merge — foundation-quality concerns, not nits.

Must-address

1. Rewrite the PR body.
Current body describes the abandoned /agents columns design with explicit "no cron, no KV cache, no scheduler" framing — the opposite of what's shipping. Frame this as "scheduler foundation + first task (Tenero token prices), reference #768." This is the highest-leverage fix; it's what every future reviewer reads.

2. Retarget to main + rebase.
Base is feat/competition-read-routes (#738). The swaps table + source column already live in main via #668, and LeaderboardClient already renders the empty-state message cleanly. The leaderboard ships independently and just goes non-empty when #738 lands. mergeable: CONFLICTING resolves with the rebase.

3. Replace readStored()'s storage.list({ prefix: "" }) with targeted batched gets.
This is the most important architectural fix. At 5 keys today it's invisible; the whole point of this DO is to grow more keys. The moment the balance task adds cursor state, every read scans all storage. The in-house pattern is x402-sponsor-relay/src/durable-objects/nonce-do.ts — targeted state.storage.get<T>(key) everywhere. Suggested shape:

const [lastRunAt, lastResult, failures, pausedUntil, nextRunAfter] = await Promise.all([
  this.ctx.storage.get<number>("lastTeneroRunAt"),
  this.ctx.storage.get<TeneroRunResult>("lastTeneroResult"),
  this.ctx.storage.get<{ tenero: number }>("consecutiveFailures"),
  this.ctx.storage.get<number>("pausedUntil"),
  this.ctx.storage.get<{ tenero?: number }>("nextRunAfter"),
]);

4. Drop resolveActiveTokenSet's D1 branch — lock the DO active-set to STATIC_TOKEN_IDS.
The leaderboard's TOKEN_DECIMALS is static (3 entries), but the DO discovers tokens dynamically from SELECT DISTINCT token_in FROM swaps. If discovery finds a token outside TOKEN_DECIMALS, the leaderboard falls back to ?? 6 decimals and silently renders a wrong USD figure with allPriced: true (no * warning). Today's static list survives because the comp's three tokens are 6 or 8 decimals — that's coincidental, not by design.

Locking the price set to the decimals table makes adding a token a deliberate two-line edit (decimals + static list + a Tenero probe), which is the right friction. When the balance task lands, it can do its own discovery — Tenero refresh stays on the static priced list.

5. Unit tests for the DO and the price/cache helpers.
The DO holds load-bearing failure-handling + backoff logic that's currently unverified. Pattern is established in x402-sponsor-relay/src/__tests__/nonce-do-sponsor-status.test.ts — prototype-method-call with hand-rolled fake state/storage doubles, no miniflare needed. Minimum five cases:

  • runTenero happy path → KV write, succeeded++
  • runTenero 5xx response → failed++, bumpFailures called
  • runTenero 429 / minuteRemaining <= 0rateLimited, nextRunAfter.tenero set to Date.now() + TENERO_RATELIMIT_BACKOFF_MS
  • tokenIdToTeneroAddress strips ::asset, passes stx through
  • getCachedTokenPrice round-trip + shape-mismatch returns null cleanly

6. Build the /api/prices endpoint referenced in lib/external/tenero/kv-cache.ts.
The file comment names /api/prices as a consumer but it doesn't exist yet. KV reads are cheap and bounded — pair it with the existing RATE_LIMIT_READ binding (300/min, already declared in wrangler.jsonc) and costs scale predictably with usage. Fits the AX surface: agents pulling current USD price refs without instantiating their own Tenero client. Suggested shape:

  • GET /api/prices — returns all cached prices: { prices: { [tokenId]: { priceUsd, fetchedAt } } }
  • GET /api/prices?token=stx — single token lookup
  • GET /api/prices with no Accept: application/json — self-doc JSON (matches the AX pattern in other routes)
  • Rate-limited via RATE_LIMIT_READ
  • Lists supported tokens (the STATIC_TOKEN_IDS set) so agents can discover what's priceable

Forward-looking, non-blocking

  • Task-dispatch will need a registry once a second task lands. The current if (teneroDue) runTenero() shape is fine for one task; at two it's a copy-paste, at three it's a smell. Worth a TODO comment marking that this evolves into for (const task of tasks) if (task.due()) await task.run() — saves a future reviewer noticing the same thing independently.
  • Per-task tick budget. When the balance task ships, give each task a bounded slice of the tick so Hiro slowness can't starve Tenero refresh and vice versa. Doesn't matter for V1.

Not concerns (clearing up review noise)

  • DO migration chain v1 (new) → v2 (deleted) → v3 (new) is the documented Cloudflare pattern for delete-and-recreate, and the comment explaining why all three tags must stay declared forever is correct.
  • DEPLOY_ENV top-level var was established in fix(rate-limit): env separation + DEPLOY_ENV + bucket rename + test handler exercise (#663) #666; not new here.
  • Inlining SchedulerDO into worker.ts as an OpenNext bundling workaround is well-documented and the build logs justify it.
  • KV cache layer is load-bearing — decouples Tenero call rate from page-view fan-out across edge regions. Stays.

@secret-mars
Copy link
Copy Markdown
Contributor

Quick acks:

Your #1 (PR body rewrite) lines up with my v266 triage of Copilot's #1 (issuecomment-4430821773) — same description-vs-scope drift. Two readers, one finding.

Your #4 (lock to STATIC_TOKEN_IDS) verified as producer/consumer silent divergence:

  • Producer (worker.ts:272-273): resolveActiveTokenSet seeds new Set(STATIC_TOKEN_IDS) then unions SELECT DISTINCT token_in FROM swaps — the D1 branch can introduce tokens absent from TOKEN_DECIMALS.
  • Consumer (app/leaderboard/page.tsx:153): TOKEN_DECIMALS[r.token_in] ?? 6 silently falls to 6.
  • allPriced (line 172) only flips false on missing price, not on decimals fallback — so today's render with allPriced: true can carry wrong USD silently if a discovered token happens to have a Tenero price.

Pattern-fit with the "two-code-paths-diverged-silently when producer ≠ consumer" shape (the same one #727 fixed with findAuthTokenInGetHandler's single-source-of-truth refactor). Your lock-down resolves it cleanly; complementary fail-loud invariant — assert DO-discovered active set ⊆ keys(TOKEN_DECIMALS); else log + skip — would surface future re-divergence in CI rather than at the USD layer.

Fair on the other items I missed at the closing APPROVE — bundling + migration arc had my read-attention. Standing by for the fixup batch; will re-review on next SHA.

@biwasxyz biwasxyz changed the base branch from feat/competition-read-routes to main May 12, 2026 15:09
biwasxyz and others added 8 commits May 12, 2026 21:06
Moves the static priced-token list out of worker.ts so the /api/prices
route (and future consumers) can import it without pulling the DO
class into Next.js's type-check pass.

The leaderboard's TOKEN_DECIMALS table is the authority on what's
priceable; the doc-comment on this constant calls out the two-step
edit rule (add to both lists, plus a Tenero probe) so future
maintainers don't get a wrong-decimals surprise.

Refs #768, addresses review item 4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lifts the Tenero refresh orchestration (fetch loop, KV writes, rate-
limit handling, structured logging) out of SchedulerDO.runTenero into
a pure function so it can be unit-tested without a DO harness or
miniflare.

The DO method now wires dependencies (kv, logger, apiKey, tokenIds)
and persists the result + failure counters to DO storage; the actual
task behavior lives in this file.

Pattern mirrors x402-sponsor-relay's split between DO orchestration
and pure task functions, per #768 review.

Refs #768, addresses review item 5 (test scaffolding).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bundles four changes from the #743 review against the SchedulerDO
class in worker.ts:

1. Lock the Tenero active-set to STATIC_TOKEN_IDS (imported from
   lib/external/tenero/tokens.ts). Drops the D1 'SELECT DISTINCT
   token_in FROM swaps' discovery branch — discovery there could
   surface tokens outside leaderboard's TOKEN_DECIMALS table, which
   would fall back to ?? 6 decimals and silently render a wrong USD
   figure with allPriced=true. Adding a token now requires a
   deliberate two-step edit; review item 4.

2. Replace readStored's storage.list({ prefix: \"\" }) with
   Promise.all targeted get<T>(key) calls. List scans every stored
   key — fine at today's 5 but the whole point of this DO is to grow
   more tasks (each with cursors + lastResult). Targeted gets keep
   read cost bounded by schema, not storage size. Pattern mirrors
   x402-sponsor-relay/durable-objects/nonce-do.ts; review item 3.

3. Slim SchedulerDO.runTenero to a thin wrapper around
   runTeneroTask() from lib/scheduler/tenero-task.ts. The DO method
   wires deps + persists results; the task body is now testable
   without the DO harness; review item 5 (scaffolding).

4. Add TODO comments near the if(teneroDue) branch (task registry
   refactor when a 2nd task lands) and near runTenero (per-task tick
   budget when balance task ships); review's forward-looking notes.

Refs #768.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reads from KV (written by SchedulerDO) so callers get sub-millisecond
responses without sharing the 100/min web-ui-ip Tenero quota. Self-docs
on Accept ≠ application/json. Supports ?token= for single-token lookup.

Closes the read side of #768 — paired with SchedulerDO's write side.
- tokenIdToTeneroAddress: literal "stx" passthrough, ::asset suffix strip,
  bare contract id unchanged
- getCachedTokenPrice/setCachedTokenPrice: null miss, round-trip with
  TTL assertion, shape-incompatible (missing fetchedAt) → null,
  non-finite priceUsd coerced to null without throwing

Addresses item 5 in whoabuddy's #743 review.
Covers the four interesting branches of the orchestration loop:
- happy path: 200 → KV write, succeeded++, headers captured
- 5xx: failed++, no KV write
- 429: rateLimited flag set, failed++
- minuteRemaining ≤ 0 on a 200: rateLimited + early break before
  remaining tokenIds are touched

Uses a Map-backed KV double and a capturing logger so the task runs
without a DO harness — the same boundary that motivated extracting
runTeneroTask in 9a99183.

Addresses item 5 in whoabuddy's #743 review.
… ref

Two Copilot review findings on #743:

1. `SUM(s.amount_in)` was typed/used as a plain number, but D1's runtime
   contract allows string returns for very large integer aggregates and
   JSON-number precision degrades past 2^53. Route the value through a
   `safeAggregateNumber` helper that BigInt-parses strings, clamps at
   `Number.MAX_SAFE_INTEGER`, and returns 0 for malformed input.

   At today's sBTC / STX scale the SUM stays well inside safe-int range,
   so this is purely defensive — under-report at the ceiling is the
   desired failure mode if a higher-decimal token enters scope.

2. The `TOKEN_DECIMALS` "keep in sync with" comment pointed to
   `lib/scheduler/scheduler-do.ts`, which doesn't exist after 9a99183 +
   b2dd3e9. `STATIC_TOKEN_IDS` now lives in `lib/external/tenero/tokens.ts`
   and is consumed by `SchedulerDO` (inline in `worker.ts`). Updated the
   pointer so the sync instruction stays actionable.
Stacks `/extended/v1/address/{addr}/balances` returns balances as decimal
strings. `Number(raw)` silently rounds past `Number.MAX_SAFE_INTEGER`
(~9e15), which for an 8-decimal asset means ~90M units before precision
degrades — well above realistic sBTC balances today but still the wrong
default parsing strategy.

New `parseSatsString` BigInt-parses the string, returns 0 for
non-numeric / negative input, and clamps at `Number.MAX_SAFE_INTEGER`
on overflow. The clamp is purely defensive — under-report at the
ceiling beats a silent rounding error.

L1 funded/spent come back as JSON numbers (mempool.space), so any
precision loss already happened inside `JSON.parse`. Left a comment
documenting the asymmetry; fixing it properly would require fetching
as text + a json-bigint pass, which is overkill for non-critical
profile-page balances.

Copilot review on #743.
Copy link
Copy Markdown
Contributor

@secret-mars secret-mars left a comment

Choose a reason for hiding this comment

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

Fixup verified — 6/6 items addressed in b2dd3e9fda3227e0 (15:21Z→15:23Z, 6 commits).

Going item by item against your 14:40Z must-address:

1. PR body rewrite ✓ — new body opens with "scheduler foundation + first task (Tenero)", refs #768 in the first paragraph, names item 6 of your review explicitly. No more "/agents table IS the leaderboard" or "no cron, no KV cache, no snapshot" framing. Constraints section cleanly explains the Workers Build RED is platform 10211 (DO migrations need wrangler deploy, not versions upload), not a code defect — matches what I'd seen empirically across 6051dbc062fb3b09a6f0ffb0.

2. Retarget to main ✓ — done at 15:09Z (your base_ref_changed); mergeable: MERGEABLE confirmed.

3. storage.list → targeted gets ✓ — readStored() (worker.ts:195) now Promise.alls 5 typed get<T> calls per known key (lastTeneroRunAt, lastTeneroResult, consecutiveFailures, pausedUntil, nextRunAfter). Pattern explicitly cites x402-sponsor-relay/src/durable-objects/nonce-do.ts in the docstring. Type annotations on every get. The comment above readStored documents why this matters as the DO grows (cost bounded by schema, not storage size) — read cost is now O(known-keys), not O(all-stored).

4. STATIC_TOKEN_IDS lockdown ✓ — resolveActiveTokenSet removed entirely (no SELECT DISTINCT token_in FROM swaps anywhere in worker.ts); static list extracted to lib/external/tenero/tokens.ts and imported on worker.ts:15. The two-code-paths-diverged-silently failure mode this prevents is now structurally impossible — adding a token is a deliberate decimals-table + static-list + Tenero-probe edit, documented in the /api/prices route comment too.

5. Unit tests ✓ — 11 cases across 3 files, all 5 cases you listed are present:

  • lib/scheduler/__tests__/tenero-task.test.ts (4 cases): happy / 5xx / 429 / minuteRemaining<=0
  • lib/external/tenero/__tests__/prices.test.ts (3 cases): 'stx' passthrough / ::asset strip / bare-contract-id passthrough
  • lib/external/tenero/__tests__/kv-cache.test.ts (4 cases): not-cached / round-trip / shape-incompatible / non-finite priceUsd (this last one is the bonus — fail-loud invariant on the consumer side beyond what you'd asked, the right defensive shape)

6. /api/prices route ✓ — app/api/prices/route.ts (188 lines): full map + ?token= single-lookup + self-doc on non-JSON Accept, RATE_LIMIT_READ (300/60s per IP), fail-closed in prod via DEPLOY_ENV !== undefined (matches #666). The "adding a new priceable token" docstring at the top of the route is a nice touch — same instruction shape lives in two surfaces now.

APPROVE. This closes the architectural-pass-after-recovery loop I'd been tracking — the v257-closing-APPROVE on 46e6badb was on a tighter scope; this fixup pass takes the substantive review you posted, applies it across 6 commits in 2 minutes of authored time, and lands cleanly. Workers Build will stay RED until the merge-to-main wrangler deploy runs the v3 migration; the /leaderboard route + SchedulerDO alarm + KV cache + /api/prices chain becomes observable post-merge.

Happy to run the post-merge verification probes (alarm fires, KV keys populate, /api/prices returns the map, /leaderboard Volume column populates) once it lands — pre-staged at daemon/scouts/743-post-merge-verify.md.

The docstring claimed we "match the full constraint string verbatim" but
the implementation uses `String#includes(...)` — a substring containment
check on the SQLite error message. Copilot flagged the mismatch on #743.

The substring approach is the deliberate trade-off: it survives runtime
wrapper-text variations across @cloudflare/workers-types releases while
still pinning to the fully-qualified `inbox_messages.payment_txid`
identifier. Rewrote the comment to describe what the code actually does
and the false-positive risk envelope.
@biwasxyz
Copy link
Copy Markdown
Contributor Author

Addressing Copilot's review (commits daf6d5e, dd54ec0, 6e5dcfe pushed):

1. Scope (PR body) — resolved
The PR description was rewritten earlier in the cycle (see commit message on the SchedulerDO line). Current body explicitly covers /leaderboard SSR rewire + KV-cached Tenero pricing + SchedulerDO as in-scope. Copilot's snapshot was the pre-rewrite version.

2. Stale lib/scheduler/scheduler-do.ts reference — fixed in daf6d5e
After 9a99183 + b2dd3e9, STATIC_TOKEN_IDS lives in lib/external/tenero/tokens.ts (consumed by SchedulerDO inline in worker.ts). Updated the "keep in sync with" pointer to match.

3. SUM(s.amount_in) precision — fixed in daf6d5e
Added safeAggregateNumber() that BigInt-parses string aggregates, clamps at Number.MAX_SAFE_INTEGER, and returns 0 for malformed input. At today's sBTC / STX decimals + comp volume range the SUM stays well inside safe-int range, so this is purely defensive — preferred failure mode is under-report at the ceiling rather than silent rounding.

4. d1-dual-write UNIQUE-violation docstring — fixed in 6e5dcfe
Rewrote the comment to describe what .includes(...) actually does — substring-match on the fully-qualified inbox_messages.payment_txid identifier, deliberately tolerant of runtime wrapper-text variations across @cloudflare/workers-types releases. False-positive risk envelope now stated explicitly.

5. BTC balance BigInt parse — fixed in dd54ec0
sBTC string balances now parsed via BigInt round-trip. L1 funded/spent come back as JSON numbers (mempool.space) — precision loss happens inside JSON.parse already, fixing it properly would need json-bigint, which I left as a comment rather than a code change since profile-page balances are non-critical.

Tests: 1017 passed. npm run build clean. cc @whoabuddy

Copy link
Copy Markdown
Contributor

@secret-mars secret-mars left a comment

Choose a reason for hiding this comment

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

Re-verify on 6e5dcfe2 — 3 new commits (daf6d5e / dd54ec0 / 6e5dcfe) addressing Copilot's findings 2–5 (your #1 already resolved by the earlier PR-body rewrite, as called out in your triage). My APPROVE on da3227e0 got dismissed by the head advance in the same ~30s window — running v124 head-SHA-pre-submit check this time. Going commit by commit:

daf6d5e (leaderboard SUM precision + stale ref):

  • safeAggregateNumber() BigInt round-trips the raw SUM(s.amount_in), clamps at Number.MAX_SAFE_INTEGER, returns 0 for non-finite / negative / non-parseable. The "honest under-report at the ceiling" framing in the docstring is the right call — silent rounding is the failure mode this prevents.
  • The BigInt(0) over 0n annotation w.r.t. tsconfig target is the kind of detail that gets lost in linter-only diffs; calling it out in-place keeps the next maintainer from "modernizing" it back to 0n and breaking the build.
  • LeaderboardJoinedRow.sum_in: number | string | null matches the new defensive shape — the runtime-boundary comment is candid about CF's docs leaving room for string returns on large aggregates.
  • Stale lib/scheduler/scheduler-do.ts pointer in the Keep in sync with comment updated to lib/external/tenero/tokens.ts consumed by SchedulerDO in worker.ts — matches the v272 extraction.

dd54ec0 (sBTC + L1 BTC sat string parse):

  • parseSatsString() mirrors the same BigInt-clamp shape from daf6d5e — DRY across two surfaces is fine; copy-paste-with-trade-off-stated beats premature shared helper.
  • L1 mempool.space side correctly notes that JSON.parse already loses precision past 2^53 — the typeof === "number" && Number.isFinite narrowing is the most you can do without a json-bigint import. The decision to leave that as a comment rather than a code change (because profile-page balances are non-critical) is the right scope discipline.
  • sBTC side: Number(raw)parseSatsString(raw) swap is a clean upgrade; same BigInt safety as the leaderboard sum.

6e5dcfe (d1-dual-write UNIQUE-violation docstring):

  • The rewrite trades verbatim-string-match framing for substring-match framing — accurate description of what .includes(...) actually does on the SQLite error message.
  • "False-positive risk envelope now stated explicitly" — agree; the bound is SQLite's own behavior (schema changes can't introduce a collision unless they reuse the inbox_messages.payment_txid column name). Honest docstring.
  • Periodic re-check note against @cloudflare/workers-types releases stays — that's the migration trigger when D1 introduces structured error codes.

APPROVE. All three commits are defensive-without-being-overzealous + trade-off-explicit-in-comments. v124 pre-submit head check at 6e5dcfe2 passed before submit this time. Workers Build will stay RED until merge-to-main wrangler deploy (platform 10211, expected per the PR body's constraints section).

Same post-merge verify offer stands — daemon/scouts/743-post-merge-verify.md pre-staged.

Copy link
Copy Markdown
Contributor

Post-review operator notes before merge:

  1. Review threads are resolved. The underlying Copilot findings are addressed in the current head (6e5dcfe).
  2. Test/Lint status: local npm test passes (1017 passed / 5 skipped); local npm run lint passes with existing image warnings. GitHub CI Test + Lint are green.
  3. Build caveat: local npm run build compiles, then fails while prerendering /leaderboard because app/leaderboard/page.tsx uses revalidate = 60 and calls getCloudflareContext({ async: true }), which falls back to Wrangler getPlatformProxy() during SSG and requires a logged-in remote proxy. That is separate from the inline SchedulerDO bundling fix. Recommendation: either confirm this is acceptable for the deployed OpenNext build environment, or make /leaderboard dynamic before merge so build-time prerender does not need Cloudflare bindings.

Post-merge verification plan:

  • Confirm main deploy uses wrangler deploy and applies DO migration v3 for SchedulerDO.
  • Visit https://aibtc.com/leaderboard once to instantiate env.SCHEDULER.idFromName("v1") and arm the first alarm.
  • Wait ~5 minutes for the first alarm tick.
  • Check logs.aibtc.com for tenero.refresh_started followed by tenero.refresh_completed with succeeded > 0.
  • Check KV for tenero:price: entries in VERIFIED_AGENTS, especially stx, sBTC, and stSTX.
  • Verify GET https://aibtc.com/api/prices with Accept: application/json returns the supported token map.
  • Verify GET https://aibtc.com/api/prices?token=stx returns a single cached entry.
  • Verify /leaderboard shows USD volume or honest partial/empty values (* / ) rather than browser-side Tenero fetches.
  • If deploy or runtime fails due the DO, rollback path is a hotfix migration v4 with deleted_classes: ["SchedulerDO"], matching the documented v2 shape.

@secret-mars
Copy link
Copy Markdown
Contributor

@whoabuddy thanks — going through your three notes:

1 + 2 (review threads / Test+Lint green): confirmed. My v273 re-APPROVE on 6e5dcfe was per-commit verification of the Copilot triage (BigInt-safe SUM, sat-string parse, d1-dual-write substring-match docstring); pairs with your "underlying Copilot findings addressed" framing.

3 (build caveat on /leaderboard prerender) — agree this should be resolved before merge, and the cleanest fix is to make the page dynamic. Reasoning:

  • The chain is revalidate = 60 → Next forces SSG prerender at build time → page imports getCachedTokenPrices → that calls getCloudflareContext({ async: true }) → falls through to getPlatformProxy() because no live request context exists at build time.
  • getPlatformProxy() reads wrangler.toml / wrangler.jsonc bindings and tries to materialize a proxy. For KV + D1 in local dev that means a remote-bound proxy requiring wrangler login. The same path runs in wrangler deploy's build container — whether that environment has the right credentials is environment-specific to the CI runner; even where it works, it's fragile (relies on the deploy job having KV/D1 read permission during build, which is wider than runtime permission needs to be).
  • export const dynamic = "force-dynamic" removes the prerender step entirely. Page evaluates getCloudflareContext inside the request lifecycle where the real bindings are attached. TTFB cost: one D1 GROUP BY + ~3 KV reads + light JS aggregation — already what an SSG-then-revalidate page is doing at revalidation time, just moved into the request path.
  • The revalidate = 60 perf gain on this surface is small: the dominant cost on /leaderboard is the D1 aggregate + KV reads (both ms-scale, both cache-friendly at the CDN edge if we want it later). Going dynamic doesn't slow the page meaningfully and removes a class of build-time-binding-resolution failures.

Concrete suggestion: drop export const revalidate = 60 from app/leaderboard/page.tsx and add export const dynamic = "force-dynamic". ~2-line change. If you'd prefer to keep ISR semantics later, the future shape is the same as the /agents route uses today — but that's a separate decision; right now the trade is build-hazard vs ~2-line dynamic flag.

On the post-merge verification plan: these mirror what I'd pre-staged at daemon/scouts/743-post-merge-verify.md (alarm → KV tenero:price: entries → /api/prices map + single-token + self-doc → /leaderboard Volume column populates). I'll run the probes against aibtc.com once the deploy lands and post the empirical outcome on this thread (or a follow-up issue if something doesn't match expectations).

Rollback shape (v4: deleted_classes: ["SchedulerDO"]): confirmed matches #772's hotfix migration shape; that's the right escape hatch and pre-cached in the constraints section of the PR body now.

If you ship the force-dynamic flip as a single follow-up commit on this PR (rather than a separate fixup PR), I can re-APPROVE on the new head with the v124 pre-submit check.

Copy link
Copy Markdown
Contributor

Follow-up after the /leaderboard build caveat:

Pushed d72559e (fix(leaderboard): render dynamically for Cloudflare bindings). This replaces revalidate = 60 with dynamic = "force-dynamic" and switches back to sync getCloudflareContext() so the page reads D1/KV/SchedulerDO only in the request lifecycle, not during build-time prerender.

Local verification on d72559e:

  • npm run lint passes (same existing image warnings only)
  • npm run test passes (1017 passed / 5 skipped)
  • npm run build passes; /leaderboard is listed as dynamic (ƒ) and no longer attempts Wrangler platform proxy during prerender

PR metadata check after push:

  • mergeable: true
  • all review threads resolved
  • Snyk success on the new head
  • Cloudflare Workers build is in progress on d72559e; if it ends in the known 10211 DO-migration limitation, that remains the expected non-prod deploy caveat documented in the PR body.

Copy link
Copy Markdown
Contributor

@secret-mars secret-mars left a comment

Choose a reason for hiding this comment

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

APPROVE on d72559e — fix is precisely what v275's build-caveat comment recommended:

  • revalidate = 60dynamic = "force-dynamic" (line 9-10)
  • getCloudflareContext({ async: true })getCloudflareContext() (line 90) — sync form restored since no build-time prerender path needs the platform proxy
  • ✅ Single load-bearing file (app/leaderboard/page.tsx); comment updated to explain the new posture

D1 GROUP BY + ~3 KV reads on every request is well within the cost budget for marginal ISR perf gain forgone (codified in v275 trade analysis). Live verifier-cron cadence (15 min) means leaderboard naturally tracks fresh state via verified-side updates, not via render-cache.

The 10211 DO-migration build caveat documented in the PR body remains the expected non-prod-deploy footnote — not a /leaderboard-specific regression.

Tests: 1017 passed / 5 skipped per whoabuddy local verification on this head. v124 head-SHA pre-submit clean (d72559e).

@whoabuddy whoabuddy merged commit db908af into main May 12, 2026
7 of 8 checks passed
@whoabuddy whoabuddy deleted the feat/agents-mcp-trades-volume branch May 12, 2026 17:52
@secret-mars
Copy link
Copy Markdown
Contributor

Deploy verified on production after merge:

GET /                    → 200
GET /leaderboard         → 200 (pre-merge baseline was 307 → /agents)
GET /agents              → 200
GET /api/leaderboard?limit=1 → 200, schema unchanged (level-scoring: rank/level/levelName/score)

/leaderboard HTML carries the new Trading Leaderboard surface — title Trading Leaderboard - AIBTC, columns Trades + Volume (USD), allPriced markers serialized. /api/leaderboard retains level-scoring semantics (123 Genesis / 828 registered / 951 total on first probe), so the new SSR page reads the new MCP-trades/Tenero-pricing surface without touching the existing API contract — separation per the PR body.

Volume column population will follow the SchedulerDO + KV-prices cron cadence (~5 min for first populate). Structural deploy is green.

The /leaderboard 200-not-307 flip is the externally-visible confirmation that the force-dynamic posture took at build time as well — no platform-proxy attempt during prerender, page composes its D1 + KV + SchedulerDO reads in the request lifecycle as designed.

Nice loop: 16:24Z build caveat raised → 16:30Z my recommendation → 17:06Z fix shipped → 17:09Z re-APPROVE → 17:52Z merge → 18:05Z deploy verified. ~1h41m end-to-end.

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.

5 participants