feat: arm live trading + daily sports scoreboard + Coinbase scaffold#57
Merged
Merged
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces multi-platform trading infrastructure: daily sports/moonshot pick lifecycle tracking (Kalshi & Polymarket), autonomous Polymarket daily sports execution, Wikipedia edit detection for Kalshi markets, awards precursor and linguistic-tell probabilistic models, platform-specific AI personas, Coinbase scaffolding (Phase 10, intentionally non-functional), extended signal types, and corresponding UI updates (Dashboard scoreboard, Connect Coinbase panel). ChangesMulti-Platform Trading Extensions
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 842881bf76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+30
to
+31
| * day-to-day. Every removed item still resolves as a deep link (the routes | ||
| * are still wired in App.tsx) — only the sidebar surface shrinks. |
There was a problem hiding this comment.
The trim removes the page routes while other UI entry points still navigate to them: CommandPalette sends “Fund account” to /funding, setupStatus uses /funding, and dashboardLanding still advertises /analytics. With the corresponding routes/pages removed from App.tsx, those clicks now fall through to NotFound, so onboarding/dashboard CTAs break rather than merely hiding sidebar items. Either keep redirect routes for the removed pages or update the remaining links to the replacement flows.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (5)
server/db.polymarket.ts (1)
111-127: 💤 Low value
closedAt: undefinedwill not clear the field if it was previously set.When
position.positionStatus !== "closed", settingclosedAt: undefinedin Drizzle's.set()will be ignored (not included in the UPDATE statement), so a previously-setclosedAttimestamp won't be cleared if a position is reopened.If positions can transition from closed back to open/closing (e.g., during reconciliation corrections), this could leave stale
closedAtvalues.Suggested fix if clearing closedAt is needed
closedAt: position.positionStatus === "closed" ? new Date() : undefined, + // To explicitly clear closedAt when not closed: + // closedAt: position.positionStatus === "closed" ? new Date() : null,Note: Drizzle treats
nullas an explicit value to set, whileundefinedis ignored.🤖 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 `@server/db.polymarket.ts` around lines 111 - 127, The update currently uses closedAt: position.positionStatus === "closed" ? new Date() : undefined which Drizzle ignores for undefined and therefore won't clear a previously-set timestamp; change the logic in the database.update(...) .set(...) for polymarketPositions to explicitly set closedAt to null when position.positionStatus !== "closed" (i.e., use null instead of undefined) so the UPDATE will clear the column when reopening a position, and verify the polymarketPositions.closedAt column allows nulls.server/polymarket-exit-monitor.test.ts (1)
43-48: ⚡ Quick winMock
getEffectivePaperTradeModedirectly per test instead of readingPAPER_TRADE_MODE.This stub hard-codes the old env-only behavior, so the suite won't catch regressions in the current per-user resolver or its fail-safe-to-paper path. Returning explicit values/errors per test would give better coverage of the safety gate this monitor now relies on.
As per coding guidelines,
Paper-mode resolution via getEffectivePaperTradeMode(userId) ... On lookup failure (DB outage), return true (paper) — never silently allow live trading on error.🤖 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 `@server/polymarket-exit-monitor.test.ts` around lines 43 - 48, The current test-wide stub for getEffectivePaperTradeMode in effectivePaperMode hard-codes behavior to the PAPER_TRADE_MODE env and masks the real per-user resolver and its fail-safe; change tests to mock getEffectivePaperTradeMode per test (using vi.fn/vi.mockImplementationOnce) to explicitly return true, false, and throw an error so you exercise live, paper, and DB-failure->paper paths; locate the mocked symbol getEffectivePaperTradeMode in the effectivePaperMode mock and replace the global env-based implementation with per-test implementations that assert a thrown error results in true (paper) to ensure the monitor's safety gate is covered.server/_core/polymarketSignalReviewer.ts (1)
584-637: 💤 Low valueDead code: intra-Claude escalation block is unreachable.
The
if (false && ...)condition makes this entire block unreachable. Per the comment, this mirrorstradingReviewer.ts, but keeping unreachable code adds maintenance burden.Consider either:
- Removing the dead block entirely, or
- Using a feature flag constant (e.g.,
const ENABLE_INTRA_ESCALATION = false) to make the intent clearerThis is low priority given the explicit comment explaining the rationale.
🤖 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 `@server/_core/polymarketSignalReviewer.ts` around lines 584 - 637, The block guarded by "if (false && !isHighStakes(stakes) && !batchResult.failed)" is dead code; remove the entire unreachable intra-model escalation block (the code that computes contestedSignals, calls callReviewer for each, and updates batchResult and escalationCitationsByMarket) or replace the literal false with a clearly named feature flag constant (e.g., ENABLE_INTRA_ESCALATION) declared near the top of polymarketSignalReviewer.ts so intent is explicit; ensure references to contestedSignals, callReviewer, batchResult.reviewsByMarket, and escalationCitationsByMarket are either removed or gated by the new flag.server/_core/polymarketPositionSync.ts (1)
306-308: 💤 Low valueSilent swallow of subscription lookup failure loses diagnostic information.
The empty
catchblock discards the error entirely. While proceeding conservatively is correct, logging the error atdebuglevel would aid troubleshooting without affecting control flow.Proposed fix
- } catch { + } catch (err) { // Subscription lookup failure is non-fatal; we proceed conservatively. + logger.debug({ err, userId }, "[PolymarketPositionSync] subscription lookup failed, proceeding anyway"); }🤖 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 `@server/_core/polymarketPositionSync.ts` around lines 306 - 308, The catch block swallowing errors after the subscription lookup in polymarketPositionSync.ts should log the caught error at debug level instead of discarding it; update the catch to accept the error (e.g., catch (err)) and call the module's existing debug logger (for example processLogger.debug or logger.debug) with a short message like "subscription lookup failed" plus the error details, falling back to console.debug if no logger is available, then continue returning/handling conservatively as before.server/polymarket-position-sync.test.ts (1)
285-314: ⚡ Quick winAssert the drift-close audit event explicitly in this test.
This scenario validates drift closure state changes, but it should also verify the required audit event emission to prevent silent regressions.
Proposed assertion addition
expect(result.closedDriftCount).toBe(1); expect(result.closedDriftPositionIds).toContain(101); + expect(mocks.logAuditEvent).toHaveBeenCalled(); + expect(mocks.logAuditEvent.mock.calls[0]?.[0]).toBe("polymarket_position_drift_closed"); // The drift-closed update should set positionStatus 'closed' + closedAt. const driftUpdate = updates.find((u) => u.positionStatus === "closed");As per coding guidelines,
server/polymarket*.ts: “Mark rowsclosedwhen absent from remote response and emitpolymarket_position_drift_closedaudit event.”🤖 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 `@server/polymarket-position-sync.test.ts` around lines 285 - 314, Add an explicit assertion that the drift-close audit event was emitted: after calling syncPolymarketPositions and checking result.closedDriftPositionIds contains 101, assert that mocks.logAuditEvent was called with the "polymarket_position_drift_closed" event (and that the event payload references the drifted position id 101 or includes the position token/market from the test). Locate the test "marks local 'open' positions absent from remote as drift-closed" and update it to verify mocks.logAuditEvent was invoked for the drifted position in addition to the existing checks on result and updates.
🤖 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 `@client/src/pages/Connect.tsx`:
- Around line 370-373: The current client-side check only ensures
trimmedApiKey/trimmedApiSecret/trimmedApiPassphrase are non-empty; add
minimum-length guards (e.g., via constants MIN_API_KEY_LENGTH,
MIN_API_SECRET_LENGTH, MIN_API_PASSPHRASE_LENGTH) and validate
trimmedApiKey.length, trimmedApiSecret.length and trimmedApiPassphrase.length
before proceeding, setting setConnectionMessage to a clear error if any are too
short; apply the same change in both places mentioned (the block using
setConnectionMessage around trimmedApiKey/trimmedApiSecret/trimmedApiPassphrase
and the similar validation at the 614-621 region) so the client rejects
too-short credentials early.
- Around line 625-629: Update any UI text that implies live/server-side
validation (e.g., the string shown when connectMutation.isPending and the
related security note text) to instead state that the client is performing a
local/format check and that true credential validation happens on first exchange
attempt; locate and change the displayed string in the Connect component where
connectMutation.isPending is used (the fragment with Loader2) and the other
identical copy in the same file (the secondary CTA/security note) to more
accurate wording about client-side format validation and deferred server-side
validation on first use.
In `@server/_core/categoryPersonas.ts`:
- Around line 83-92: The returned personas currently spread PROFIT_PERSONA_BASE
but only override platform and category, leaving Kalshi-specific fields (e.g.,
id and systemMandate) unchanged; update getCategoryPersona and
listPersonasForPlatform so they derive/override id and systemMandate based on
the platform (and category when appropriate) instead of inheriting
PROFIT_PERSONA_BASE values—ensure the new id reflects the platform (e.g.,
"polymarket.profit-reviewer" vs "kalshi.profit-reviewer") and that
systemMandate/fee/exchange instructions are replaced with platform-appropriate
text when constructing the returned CategoryPersona objects.
In `@server/_core/crossPlatformArbitrage.ts`:
- Around line 247-264: The code currently only rejects non-finite or <=0 prices,
which allows out-of-range values like 1, 1.5 or cent‑scaled numbers to produce
bogus spreads; update the guards around kalshi.yesPrice and pm.yesPrice (in the
loop using kalshiMarkets, polymarketMarkets, findBestPolymarketMatch, pm,
minLiquidity, minSpread) to require a valid probability interval (e.g.
Number.isFinite(price) && price > 0 && price < 1) before calling
findBestPolymarketMatch or computing spread so both Kalshi and Polymarket prices
are gated to (0,1).
In `@server/_core/distributedLock.ts`:
- Around line 222-230: The new createPolymarketAutonomousTradingLock creates a
separate namespace and allows concurrent autonomy loops; change it to reuse the
existing per-user autonomy lock key (the same key used by
createAutonomousTradingLock) or make it acquire a higher-level umbrella lock
before taking the platform-specific lock so both Kalshi and Polymarket cannot
run concurrently for the same user; update createPolymarketAutonomousTradingLock
to either return new DistributedLock with the identical key string used in
createAutonomousTradingLock or modify callers to acquire a shared umbrella
DistributedLock (e.g., "autonomous_trading_user_${userId}") before creating
platform-specific locks to preserve the single in-flight guarantee.
In `@server/_core/grokPersonas.ts`:
- Around line 68-78: The current functions getGrokPersona and
listGrokPersonasForPlatform are just spreading PROFIT_GROK_PERSONA and
overriding platform, causing a mismatch between the platform field and the
embedded systemMandate (which references Kalshi). Update these functions to
return a persona whose systemMandate aligns with the platform: either remove the
blanket platform override and make PROFIT_GROK_PERSONA platform-agnostic, or
create per-platform persona variants (e.g., PROFIT_GROK_PERSONA_KALSHI,
PROFIT_GROK_PERSONA_POLYMARKET) and return the matching one based on the
platform parameter; ensure getGrokPersona and listGrokPersonasForPlatform set
both platform and systemMandate consistently rather than only changing platform.
In `@server/_core/paperTrading.ts`:
- Around line 303-461: Add an audit entry for simulated Polymarket orders inside
simulatePolymarketOrderFill: after successfully inserting into polymarketOrders
(where clientOrderId is created) write an audit record (e.g.,
db.insert(auditLogs).values(...)) or invoke the existing audit helper with
fields userId/scopedUserId, clientOrderId, marketId, tokenId, side/positionSide,
sizeUsdc, limitPrice/fillPrice, status:"filled", and simulated:true; ensure the
audit write is attempted before returning success and log errors similarly to
the existing logger.error blocks so simulated orders are fully auditable.
In `@server/_core/polymarketAuth.ts`:
- Around line 234-239: The catch block currently logs and returns the raw caught
error (logger.error({ err: error }, "[Polymarket] Place order failed") and
error.message) which can leak credentials; update the catch in polymarketAuth so
you do NOT log or return the raw error object or its message: log a
redacted/normalized entry (e.g., logger.error("[Polymarket] Place order failed -
redacted error") optionally with a short non-sensitive errorId) and return a
generic caller-facing message (e.g., { success: false, error: "Failed to place
order" }) instead of error.message; ensure the error variable is not serialized
anywhere and, if you need internal diagnostics, store full details in a secure
audit sink tied to the errorId.
- Around line 188-240: placePolymarketOrder currently places real exchange
orders but doesn't emit audit records; modify it to accept or obtain an
identity/openId and call db.logAuditEvent for both success and failure paths.
Specifically, thread an openId (or a minimal identity context) into
placePolymarketOrder signature (or fetch from the caller) and, after calling
buildPolymarketClobClient/submitSignedPolymarketOrder, call
db.logAuditEvent("polymarket.order.placed", { tokenId, side, price, size,
orderId: <returned id>, clientMeta: {...} }, openId) on success and
db.logAuditEvent("polymarket.order.failed", { tokenId, side, price, size, error:
<error message>, stack?: <error stack> }, openId) in the catch block; ensure
payloads contain the request inputs and outcome, and keep calls next to
submitSignedPolymarketOrder and in the catch so all placements and failures are
audited.
In `@server/_core/polymarketClusterMonitor.ts`:
- Line 350: The property assignment for fadeTriggerReady uses a redundant
boolean expression "retracement.detected && retracement.detected" — replace it
with the single value "retracement.detected" in the object where
fadeTriggerReady is set (locate the assignment in polymarketClusterMonitor.ts
around the retracement usage) to remove the copy‑paste artifact and keep the
same boolean semantics.
In `@server/_core/polymarketExitMonitor.ts`:
- Around line 113-129: buildTokenPriceMap currently treats an empty result from
fetchPolymarketMarkets as a valid (but empty) map which later causes code to
fall back to stale position.currentPrice; change buildTokenPriceMap to treat
no-data as a failure by returning null when fetchPolymarketMarkets returns an
empty array or when the constructed map has size 0 (i.e., after creating map, if
markets.length === 0 || map.size === 0 return null), keeping the existing catch
behavior; apply the same null-on-empty logic to the other analogous helper/usage
referenced in the diff (the block that also constructs/returns a price map) so
callers can detect no-data and skip the tick instead of using stale fallback
prices.
In `@server/_core/polymarketMarketMaking.ts`:
- Around line 155-158: After merging caller params into p (MMParams) in the
quote-generation function that returns MMQuotePair, validate and sanitize p:
clamp numeric fields (use clamp) such as inventoryRatio to [-1,1], fairValue
bounds to [0.02,0.98], orderSizeUsdc to >= 0, and enforce documented min/max
ranges for spreads, gamma, time, and volatility; if a min > max for any spread
bound swap or reset to DEFAULT_PARAMS and log a warning. Apply the same
sanitization logic where params are merged later (the blocks around the existing
merges you noted at 167-173 and 189-198) so all downstream quote math uses safe,
bounded values.
- Around line 293-315: The filter uses priceSum < 1 - minProfitPct but the code
computes guaranteedProfitPct = (1 - priceSum) / priceSum and returns that, so
make the predicate consistent: compute guaranteedProfitPct from priceSum and
replace the priceSum check with guaranteedProfitPct >= minProfitPct (and ensure
you still skip non-finite/<=0 prices); update the block around priceSum,
guaranteedProfitPct, and the results.push to use this new condition so
minProfitPct aligns with the returned guaranteedProfitPct.
In `@server/_core/polymarketSignals.ts`:
- Around line 531-534: The code calls buildFadeRecommendations(clusterSignals,
0.5) with a hardcoded currentImpliedProbability which causes fadeSide and
suggestedLimitPrice to be calculated from the wrong probability; update the call
so buildFadeRecommendations is invoked per-market with each market's actual
implied probability (or refactor buildFadeRecommendations to accept a map of
per-market probabilities) so that fadeSide and suggestedLimitPrice use the real
per-market probability; locate references to buildFadeRecommendations,
currentImpliedProbability, fadeSide and suggestedLimitPrice in the monitor logic
and ensure the per-market probability is passed through the call chain instead
of the placeholder 0.5 (or alternatively change the surrounding comment to
accurately state that 0.5 is a placeholder if you choose not to fix the logic).
In `@server/_core/polymarketSigner.ts`:
- Around line 100-151: The submitSignedPolymarketOrder function currently calls
client.postOrder(...) directly; wrap that call (and future Polymarket API calls)
with a resilience layer: implement a retry-with-exponential-backoff that retries
on transient HTTP failures (status codes 5xx, 408, 425, 429) with jitter and a
sensible max attempts, and combine it with a circuit breaker that trips after 5
failures within 30 seconds and stays open (fails-fast) for 30 seconds; extract
this into a helper (e.g., postOrderWithResilience or a generic
callWithResilience) and replace the direct client.postOrder call in
submitSignedPolymarketOrder with that helper so failures and retries follow the
required policy while preserving existing response/error handling.
In `@server/db.polymarket-credentials.ts`:
- Around line 162-178: The deletePolymarketCredentials function currently
deletes records without emitting an audit entry; add an audit log call after
successful deletion (and before returning) that records the userId, action
("delete_polymarket_credentials"), timestamp, and the actor/context if
available; use the existing logger or the project's audit logging utility
(referencing deletePolymarketCredentials and polymarketCredentials) and ensure
the audit entry is recorded in the same try block before returning { success:
true } so failures still propagate to the catch where the existing error log
remains.
- Around line 47-84: The save operation for Polymarket credentials (the
database.insert(polymarketCredentials)...onConflictDoUpdate block) needs an
audit entry via db.logAuditEvent after a successful upsert; update the function
that performs this work to accept an openId parameter, and after the
insert/upsert returns success call db.logAuditEvent (or database.logAuditEvent)
with a descriptive event name (e.g., "polymarket.credentials.saved"), the target
userId, the actor openId, and metadata indicating which sensitive fields were
provided/overwritten (e.g., whether encryptedWalletPrivateKey and walletAddress
were included and the signatureType), ensuring the audit call happens before
returning { success: true } and that errors continue to be logged/propagated as
before.
In `@server/polymarket.autonomy.e2e.test.ts`:
- Around line 145-154: The test currently embeds a synthetic-looking hex private
key in the test object (walletPrivateKey) which triggers GitGuardian; replace
that hardcoded hex value with an explicit, non-secret test constant or
placeholder (e.g., TEST_WALLET_PRIVATE_KEY) used in the same test setup where
walletPrivateKey is set and
mocks.getKalshiCapital/mocks.getPolymarketPerformanceOverview are configured, or
read it from a clearly non-secret env var for tests; alternatively add a
.gitguardian.yaml exclusion for this test file pattern to silence the alert.
In `@server/polymarket.cluster.test.ts`:
- Around line 219-226: The test currently wraps the assertions in an if
(copySignals.length > 0) block so it silently passes when no copy signals are
produced; change the test to explicitly assert that copySignals exist and then
run the recommendation checks: add an assertion like
expect(copySignals.length).toBeGreaterThan(0) (or fail the test if zero) before
calling buildFadeRecommendations, then remove the conditional and assert copyRec
is defined and its side is "yes" using the existing buildFadeRecommendations and
recs.find logic.
In `@server/routers.ts`:
- Around line 2752-2761: The input credential fields (input.apiKey,
input.apiSecret, input.apiPassphrase, input.walletPrivateKey,
input.walletAddress, input.signatureType) are validated using trim() but the raw
values are being saved; trim all those fields before calling
polymarketCredDb.savePolymarketCredentials so you persist sanitized values.
Update the call site that invokes polymarketCredDb.savePolymarketCredentials to
pass trimmed strings (e.g., use input.apiKey.trim(), input.apiSecret.trim(),
input.apiPassphrase.trim(), and trim
walletPrivateKey/walletAddress/signatureType) so stored credentials match
validation.
---
Nitpick comments:
In `@server/_core/polymarketPositionSync.ts`:
- Around line 306-308: The catch block swallowing errors after the subscription
lookup in polymarketPositionSync.ts should log the caught error at debug level
instead of discarding it; update the catch to accept the error (e.g., catch
(err)) and call the module's existing debug logger (for example
processLogger.debug or logger.debug) with a short message like "subscription
lookup failed" plus the error details, falling back to console.debug if no
logger is available, then continue returning/handling conservatively as before.
In `@server/_core/polymarketSignalReviewer.ts`:
- Around line 584-637: The block guarded by "if (false && !isHighStakes(stakes)
&& !batchResult.failed)" is dead code; remove the entire unreachable intra-model
escalation block (the code that computes contestedSignals, calls callReviewer
for each, and updates batchResult and escalationCitationsByMarket) or replace
the literal false with a clearly named feature flag constant (e.g.,
ENABLE_INTRA_ESCALATION) declared near the top of polymarketSignalReviewer.ts so
intent is explicit; ensure references to contestedSignals, callReviewer,
batchResult.reviewsByMarket, and escalationCitationsByMarket are either removed
or gated by the new flag.
In `@server/db.polymarket.ts`:
- Around line 111-127: The update currently uses closedAt:
position.positionStatus === "closed" ? new Date() : undefined which Drizzle
ignores for undefined and therefore won't clear a previously-set timestamp;
change the logic in the database.update(...) .set(...) for polymarketPositions
to explicitly set closedAt to null when position.positionStatus !== "closed"
(i.e., use null instead of undefined) so the UPDATE will clear the column when
reopening a position, and verify the polymarketPositions.closedAt column allows
nulls.
In `@server/polymarket-exit-monitor.test.ts`:
- Around line 43-48: The current test-wide stub for getEffectivePaperTradeMode
in effectivePaperMode hard-codes behavior to the PAPER_TRADE_MODE env and masks
the real per-user resolver and its fail-safe; change tests to mock
getEffectivePaperTradeMode per test (using vi.fn/vi.mockImplementationOnce) to
explicitly return true, false, and throw an error so you exercise live, paper,
and DB-failure->paper paths; locate the mocked symbol getEffectivePaperTradeMode
in the effectivePaperMode mock and replace the global env-based implementation
with per-test implementations that assert a thrown error results in true (paper)
to ensure the monitor's safety gate is covered.
In `@server/polymarket-position-sync.test.ts`:
- Around line 285-314: Add an explicit assertion that the drift-close audit
event was emitted: after calling syncPolymarketPositions and checking
result.closedDriftPositionIds contains 101, assert that mocks.logAuditEvent was
called with the "polymarket_position_drift_closed" event (and that the event
payload references the drifted position id 101 or includes the position
token/market from the test). Locate the test "marks local 'open' positions
absent from remote as drift-closed" and update it to verify mocks.logAuditEvent
was invoked for the drifted position in addition to the existing checks on
result and updates.
🪄 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: d18b9f22-82ac-40b2-94ac-59c727afc1be
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
.env.exampleclient/src/App.tsxclient/src/components/shell/LoginScreen.tsxclient/src/components/shell/navigation.tsclient/src/pages/Analytics.tsxclient/src/pages/Chat.tsxclient/src/pages/Connect.tsxclient/src/pages/Dashboard.tsxclient/src/pages/Funding.tsxclient/src/pages/PortfolioOptimization.tsxclient/src/pages/SentimentAnalysis.tsxclient/src/pages/Strategies.tsxclient/src/pages/TradingReadiness.tsxdrizzle/migrations/0011_polymarket_full_tables.sqldrizzle/migrations/0012_polymarket_signing.sqldrizzle/schema.tspackage.jsonserver/_core/categoryPersonas.tsserver/_core/crossPlatformArbitrage.tsserver/_core/distributedLock.tsserver/_core/env.tsserver/_core/grokPersonas.tsserver/_core/index.tsserver/_core/paperTrading.tsserver/_core/polymarketAuth.tsserver/_core/polymarketAutonomy.tsserver/_core/polymarketClusterMonitor.tsserver/_core/polymarketExitMonitor.tsserver/_core/polymarketLearning.tsserver/_core/polymarketMarketMaking.tsserver/_core/polymarketPositionSync.tsserver/_core/polymarketRisk.tsserver/_core/polymarketSignalReviewer.tsserver/_core/polymarketSignals.tsserver/_core/polymarketSigner.tsserver/_core/schedulerHeartbeat.tsserver/chat.router.tsserver/cross-platform-arbitrage.test.tsserver/db.chat.tsserver/db.desk-memory.tsserver/db.polymarket-credentials.tsserver/db.polymarket.tsserver/db.tsserver/polymarket-exit-monitor.test.tsserver/polymarket-paper-mode.test.tsserver/polymarket-position-sync.test.tsserver/polymarket.autonomy.e2e.test.tsserver/polymarket.autonomy.mutex.test.tsserver/polymarket.cluster.test.tsserver/polymarket.learning.test.tsserver/polymarket.risk.test.tsserver/polymarket.signals.test.tsserver/routers.ts
💤 Files with no reviewable changes (9)
- client/src/pages/Analytics.tsx
- client/src/pages/PortfolioOptimization.tsx
- server/db.chat.ts
- server/chat.router.ts
- client/src/pages/TradingReadiness.tsx
- client/src/pages/Strategies.tsx
- client/src/pages/Funding.tsx
- client/src/pages/Chat.tsx
- client/src/pages/SentimentAnalysis.tsx
Comment on lines
+222
to
+230
| /** | ||
| * Create a distributed lock for Polymarket autonomous trading per user. | ||
| * Mirrors createAutonomousTradingLock but in a separate namespace so a | ||
| * Polymarket run never blocks on a Kalshi run for the same user (and vice | ||
| * versa). | ||
| */ | ||
| export function createPolymarketAutonomousTradingLock(userId: number): DistributedLock { | ||
| return new DistributedLock(`polymarket_autonomous_trading_user_${userId}`); | ||
| } |
There was a problem hiding this comment.
Keep autonomy locking shared across platforms.
This new namespace lets a user's Kalshi and Polymarket autonomy loops run concurrently, which drops the repo's single in-flight autonomy guarantee and reopens races around shared budget/risk state. Reuse the existing per-user autonomy key here, or add a higher-level umbrella lock that both platforms must take first.
As per coding guidelines, server/_core/*.ts: "Distributed lock: distributedLock.ts uses Postgres to ensure only one autonomy run per user is in-flight at a time."
🤖 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 `@server/_core/distributedLock.ts` around lines 222 - 230, The new
createPolymarketAutonomousTradingLock creates a separate namespace and allows
concurrent autonomy loops; change it to reuse the existing per-user autonomy
lock key (the same key used by createAutonomousTradingLock) or make it acquire a
higher-level umbrella lock before taking the platform-specific lock so both
Kalshi and Polymarket cannot run concurrently for the same user; update
createPolymarketAutonomousTradingLock to either return new DistributedLock with
the identical key string used in createAutonomousTradingLock or modify callers
to acquire a shared umbrella DistributedLock (e.g.,
"autonomous_trading_user_${userId}") before creating platform-specific locks to
preserve the single in-flight guarantee.
Comment on lines
+47
to
+84
| try { | ||
| await database | ||
| .insert(polymarketCredentials) | ||
| .values({ | ||
| userId, | ||
| apiKeyEncrypted: encryptedApiKey, | ||
| apiSecretEncrypted: encryptedApiSecret, | ||
| apiPassphraseEncrypted: encryptedApiPassphrase, | ||
| walletPrivateKeyEncrypted: encryptedWalletPrivateKey, | ||
| walletAddress, | ||
| signatureType, | ||
| accountStatus: "connected", | ||
| lastSyncedAt: new Date(), | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: polymarketCredentials.userId, | ||
| set: { | ||
| apiKeyEncrypted: encryptedApiKey, | ||
| apiSecretEncrypted: encryptedApiSecret, | ||
| apiPassphraseEncrypted: encryptedApiPassphrase, | ||
| // Only overwrite wallet fields when the caller actually supplied | ||
| // them. Lets a credential rotation that only touches API key | ||
| // leave the existing wallet-key untouched. | ||
| ...(encryptedWalletPrivateKey | ||
| ? { walletPrivateKeyEncrypted: encryptedWalletPrivateKey } | ||
| : {}), | ||
| ...(walletAddress ? { walletAddress } : {}), | ||
| ...(Number.isFinite(options.signatureType) ? { signatureType } : {}), | ||
| accountStatus: "connected", | ||
| lastSyncedAt: new Date(), | ||
| }, | ||
| }); | ||
|
|
||
| return { success: true }; | ||
| } catch (error) { | ||
| logger.error({ err: error }, "[Database] Save Polymarket credentials failed"); | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
Missing audit logging for credential save operations.
Per coding guidelines, significant events should be logged via db.logAuditEvent. Saving Polymarket credentials (especially with wallet private keys) is a security-sensitive operation that should be audited.
Proposed fix to add audit logging
return { success: true };
} catch (error) {
logger.error({ err: error }, "[Database] Save Polymarket credentials failed");
throw error;
}
+
+ // After successful save, log audit event (import logAuditEvent from ./db)
+ // await logAuditEvent("polymarket_credentials_saved", { hasWalletKey: !!encryptedWalletPrivateKey }, openId);
}Note: You'll need to pass the openId parameter to enable proper audit attribution.
🤖 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 `@server/db.polymarket-credentials.ts` around lines 47 - 84, The save operation
for Polymarket credentials (the
database.insert(polymarketCredentials)...onConflictDoUpdate block) needs an
audit entry via db.logAuditEvent after a successful upsert; update the function
that performs this work to accept an openId parameter, and after the
insert/upsert returns success call db.logAuditEvent (or database.logAuditEvent)
with a descriptive event name (e.g., "polymarket.credentials.saved"), the target
userId, the actor openId, and metadata indicating which sensitive fields were
provided/overwritten (e.g., whether encryptedWalletPrivateKey and walletAddress
were included and the signatureType), ensuring the audit call happens before
returning { success: true } and that errors continue to be logged/propagated as
before.
Comment on lines
+162
to
+178
| export async function deletePolymarketCredentials(userId: number) { | ||
| const database = await getDb(); | ||
| if (!database) { | ||
| throw new Error("Database not initialized"); | ||
| } | ||
|
|
||
| try { | ||
| await database | ||
| .delete(polymarketCredentials) | ||
| .where(eq(polymarketCredentials.userId, userId)); | ||
|
|
||
| return { success: true }; | ||
| } catch (error) { | ||
| logger.error({ err: error }, "[Database] Delete Polymarket credentials failed"); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing audit logging for credential deletion.
Deleting Polymarket credentials is a security-sensitive operation that should be audited per coding guidelines.
🤖 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 `@server/db.polymarket-credentials.ts` around lines 162 - 178, The
deletePolymarketCredentials function currently deletes records without emitting
an audit entry; add an audit log call after successful deletion (and before
returning) that records the userId, action ("delete_polymarket_credentials"),
timestamp, and the actor/context if available; use the existing logger or the
project's audit logging utility (referencing deletePolymarketCredentials and
polymarketCredentials) and ensure the audit entry is recorded in the same try
block before returning { success: true } so failures still propagate to the
catch where the existing error log remains.
Comment on lines
+145
to
+154
| apiPassphrase: "p", | ||
| walletPrivateKey: "0x" + "11".repeat(32), | ||
| walletAddress: "0x0000000000000000000000000000000000000001", | ||
| signatureType: 1, | ||
| }); | ||
| mocks.getKalshiCapital.mockResolvedValue({ | ||
| currentBalance: 500, | ||
| startingBalance: 500, | ||
| }); | ||
| mocks.getPolymarketPerformanceOverview.mockResolvedValue({ |
There was a problem hiding this comment.
GitGuardian false positive: synthetic test credentials triggering secret detection.
The pipeline failure flags walletPrivateKey: "0x" + "11".repeat(32) as a hardcoded secret. This is an obviously synthetic test key (64 repeated '1' characters) that cannot represent real funds. However, to silence the alert and improve clarity:
Suggested fix to avoid GitGuardian alert
- walletPrivateKey: "0x" + "11".repeat(32),
- walletAddress: "0x0000000000000000000000000000000000000001",
+ walletPrivateKey: `0x${"a1".repeat(32)}`, // synthetic test key
+ walletAddress: "0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF", // checksum-valid placeholderAlternatively, add a .gitguardian.yaml exclusion for this test file pattern.
🤖 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 `@server/polymarket.autonomy.e2e.test.ts` around lines 145 - 154, The test
currently embeds a synthetic-looking hex private key in the test object
(walletPrivateKey) which triggers GitGuardian; replace that hardcoded hex value
with an explicit, non-secret test constant or placeholder (e.g.,
TEST_WALLET_PRIVATE_KEY) used in the same test setup where walletPrivateKey is
set and mocks.getKalshiCapital/mocks.getPolymarketPerformanceOverview are
configured, or read it from a clearly non-secret env var for tests;
alternatively add a .gitguardian.yaml exclusion for this test file pattern to
silence the alert.
Comment on lines
+219
to
+226
| const copySignals = signals.filter((s) => s.strategy === "copy"); | ||
| if (copySignals.length > 0) { | ||
| const recs = buildFadeRecommendations(copySignals, 0.008); | ||
| const copyRec = recs.find((r) => r.action === "copy_buy"); | ||
| expect(copyRec).toBeDefined(); | ||
| expect(copyRec?.side).toBe("yes"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Conditional assertion may silently pass without testing the intended behavior.
If detectClusterActivity returns no copy signals (due to a bug or threshold change), this test passes silently without verifying the copy_buy recommendation logic.
Proposed fix to ensure the test fails if no copy signals are generated
const copySignals = signals.filter((s) => s.strategy === "copy");
- if (copySignals.length > 0) {
- const recs = buildFadeRecommendations(copySignals, 0.008);
- const copyRec = recs.find((r) => r.action === "copy_buy");
- expect(copyRec).toBeDefined();
- expect(copyRec?.side).toBe("yes");
- }
+ expect(copySignals.length).toBeGreaterThan(0);
+ const recs = buildFadeRecommendations(copySignals, 0.008);
+ const copyRec = recs.find((r) => r.action === "copy_buy");
+ expect(copyRec).toBeDefined();
+ expect(copyRec?.side).toBe("yes");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const copySignals = signals.filter((s) => s.strategy === "copy"); | |
| if (copySignals.length > 0) { | |
| const recs = buildFadeRecommendations(copySignals, 0.008); | |
| const copyRec = recs.find((r) => r.action === "copy_buy"); | |
| expect(copyRec).toBeDefined(); | |
| expect(copyRec?.side).toBe("yes"); | |
| } | |
| }); | |
| const copySignals = signals.filter((s) => s.strategy === "copy"); | |
| expect(copySignals.length).toBeGreaterThan(0); | |
| const recs = buildFadeRecommendations(copySignals, 0.008); | |
| const copyRec = recs.find((r) => r.action === "copy_buy"); | |
| expect(copyRec).toBeDefined(); | |
| expect(copyRec?.side).toBe("yes"); |
🤖 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 `@server/polymarket.cluster.test.ts` around lines 219 - 226, The test currently
wraps the assertions in an if (copySignals.length > 0) block so it silently
passes when no copy signals are produced; change the test to explicitly assert
that copySignals exist and then run the recommendation checks: add an assertion
like expect(copySignals.length).toBeGreaterThan(0) (or fail the test if zero)
before calling buildFadeRecommendations, then remove the conditional and assert
copyRec is defined and its side is "yes" using the existing
buildFadeRecommendations and recs.find logic.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee95d47887
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+619
to
+622
| const rawSize = estimateSizeForRiskBudget( | ||
| bankroll, | ||
| best.fairValueEstimate, | ||
| best.limitPrice, |
There was a problem hiding this comment.
When the selected candidate is a NO token, fairValueEstimate is still the YES probability from the signal, but estimateSizeForRiskBudget treats this argument as the probability that the purchased token wins. This reverses Kelly sizing for every best.side === "no" candidate (for example, a contrarian NO on a 93% YES market can be sized as if NO has ~80% win probability), so live Polymarket orders can be oversized despite the signal's actual edge. Pass best.side === "no" ? 1 - best.fairValueEstimate : best.fairValueEstimate here before sizing.
Useful? React with 👍 / 👎.
Comment on lines
+210
to
+215
| if (!order.walletPrivateKey || !order.walletAddress) { | ||
| return { | ||
| success: false, | ||
| error: "Polymarket wallet private key + funder address required for order signing", | ||
| }; | ||
| } |
There was a problem hiding this comment.
When POLYMARKET_OWNER_ADDRESS is unset, this path still submits live CLOB orders as long as per-user wallet credentials exist, but syncPolymarketPositions() returns POLYMARKET_OWNER_ADDRESS not set and the live order path does not insert a local polymarketPositions row. In that deployment, newly opened live positions are invisible to the dashboard and exit monitor, so auto-close/trailing-stop logic never sees them; require the owner address (or otherwise persist the opened position) before allowing live placement.
Useful? React with 👍 / 👎.
mikelaurenzo7-collab
pushed a commit
that referenced
this pull request
May 9, 2026
…wner address Two correctness fixes flagged by Codex review on PR #57. P1 — polymarketAutonomy.ts NO-side sizing: estimateSizeForRiskBudget() expects the win probability of the *purchased* token, but the autonomy loop was always passing `best.fairValueEstimate` which is the YES probability. For `best.side === "no"` candidates that reverses Kelly: a contrarian NO on a 93% YES market got sized as if NO had ~80% win probability, producing wildly oversized live orders. Flip the probability for NO before sizing. P2 — polymarketAuth.ts placement gate: When POLYMARKET_OWNER_ADDRESS is unset, syncPolymarketPositions() silently no-ops. Live orders still went through, but the exit monitor never saw the resulting positions — no auto-close, no trailing stop, no drift-close. Block live placement until the env var is set. Failing closed beats trading positions we can't reconcile. https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
server/kalshi.linguistic-tells.test.ts (1)
125-138: ⚡ Quick winAssert the exact posterior cap used by the implementation.
This test currently allows values up to
0.95, but the implementation caps at0.92. Tightening this assertion will catch regressions earlier.Proposed test tightening
- it("posterior is capped below 0.95 even with many tells", () => { + it("posterior is capped at 0.92 even with many tells", () => { @@ - expect(result!.posterior).toBeLessThanOrEqual(0.95); + expect(result!.posterior).toBeLessThanOrEqual(0.92); });🤖 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 `@server/kalshi.linguistic-tells.test.ts` around lines 125 - 138, Update the test assertion to reflect the implementation cap of 0.92: in the test calling lookupLinguisticTellPrior (the "posterior is capped..." spec) replace expect(result!.posterior).toBeLessThanOrEqual(0.95) with an assertion that uses 0.92 (e.g., toBeLessThanOrEqual(0.92) or toBeCloseTo(0.92) if you want strictness for floating errors) so the test matches the actual cap in lookupLinguisticTellPrior.
🤖 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 `@server/_core/grokPersonas.ts`:
- Around line 89-95: getGrokPersona currently ignores the passed-in _category
and always returns a cached persona with category "other"; update getGrokPersona
to preserve the caller’s category by returning a persona that copies the
selected base persona (KALSHI_GROK_PERSONA or POLYMARKET_GROK_PERSONA) but
overrides its category with the provided category parameter (ensure you use the
function parameter name, e.g., category, not the unused _category). Locate
getGrokPersona and modify the return to merge/copy the chosen persona and set
category to the caller-supplied value so downstream logging/analytics see the
correct category.
- Around line 51-53: Remove the `${RULES_BLOCK}` placeholder from the cached
Grok mandate so callers cannot accidentally re-insert the full rules; locate the
cached mandate string/array in server/_core/grokPersonas.ts (the lines that
currently include the token) and delete that token/line fragment, leaving the
mandate text without the `${RULES_BLOCK}` placeholder so
injectVerbatimRulesBlock() remains the only code path that can splice rulesText
into prompts.
In `@server/_core/kalshiSignals.ts`:
- Around line 29-41: calculateSignalPerformance can crash because
performance.get(signal.signalType)! assumes the map was pre-seeded for every
SignalType; update the function to lazily initialize entries when a signalType
is first encountered instead of relying only on the initial seed. Concretely,
inside calculateSignalPerformance (where performance.get(signal.signalType) is
used) check performance.has(signal.signalType) and if missing create and set a
default aggregate object (same shape as existing seeded entries) before calling
performance.get(...), or factor this into a small helper like
getOrCreatePerformance(signal.signalType) to centralize initialization;
reference SignalType and performance.get(signal.signalType) when making the
change.
In `@server/_core/wikipediaEditWatcher.ts`:
- Around line 35-40: The circuit breaker configuration for wikiBreaker uses a
60s window and cooldown but must match the project's 30s/30s resilience profile;
update the CircuitBreaker instantiation for wikiBreaker (symbol: wikiBreaker) to
set windowMs to 30_000 and cooldownMs to 30_000 while keeping failureThreshold
at 5 and name "wikipedia.edit-watcher" so it trips after 5 failures in 30s and
fails fast for 30s.
- Around line 209-211: The current cursor update can be rolled back by slower
overlapping poll cycles because it blindly sets state.cursors.set(page.title,
newest.timestamp) from out.reduce; change it to be monotonic by reading the
existing cursor (state.cursors.get(page.title) or a zero/epoch default) and only
advancing it to the greater of the two timestamps (compare existing vs
newest.timestamp and set the max), so overlapping older cycles cannot overwrite
a newer cursor.
- Around line 283-294: matchSignalsToMarkets currently matches purely by keyword
which causes false positives; update it to require a category match when
category data exists. In matchSignalsToMarkets, after computing kw and title,
check market.category and the signal's category (e.g., sig.category or
sig.marketCategory) and only push {signal, market} if either no category is
present on both sides or the normalized categories are equal; otherwise skip the
pair. Ensure category comparisons are case-insensitive and keep the existing
keyword include check as a fallback when categories are missing.
---
Nitpick comments:
In `@server/kalshi.linguistic-tells.test.ts`:
- Around line 125-138: Update the test assertion to reflect the implementation
cap of 0.92: in the test calling lookupLinguisticTellPrior (the "posterior is
capped..." spec) replace expect(result!.posterior).toBeLessThanOrEqual(0.95)
with an assertion that uses 0.92 (e.g., toBeLessThanOrEqual(0.92) or
toBeCloseTo(0.92) if you want strictness for floating errors) so the test
matches the actual cap in lookupLinguisticTellPrior.
🪄 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: b8656065-1625-4381-b3fd-c6fca191a4c9
📒 Files selected for processing (23)
client/src/components/CommandPalette.tsxclient/src/lib/dashboardLanding.test.tsclient/src/lib/dashboardLanding.tsclient/src/lib/setupStatus.tsserver/_core/categoryPersonas.tsserver/_core/crossPlatformArbitrage.tsserver/_core/grokPersonas.tsserver/_core/index.tsserver/_core/kalshiAwardsPrecursor.tsserver/_core/kalshiLinguisticTells.tsserver/_core/kalshiSignals.tsserver/_core/mlSignalEnsemble.tsserver/_core/polymarketAuth.tsserver/_core/polymarketAutonomy.tsserver/_core/polymarketClusterMonitor.tsserver/_core/polymarketMarketMaking.tsserver/_core/polymarketSignals.tsserver/_core/wikipediaEditWatcher.tsserver/kalshi.awards-precursor.test.tsserver/kalshi.linguistic-tells.test.tsserver/ml-ensemble.test.tsserver/routers.tsserver/wikipedia-edit-watcher.test.ts
✅ Files skipped from review due to trivial changes (1)
- client/src/lib/dashboardLanding.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- server/_core/polymarketMarketMaking.ts
- server/_core/polymarketAuth.ts
- server/_core/index.ts
- server/_core/crossPlatformArbitrage.ts
- server/_core/polymarketSignals.ts
- server/routers.ts
- server/_core/polymarketClusterMonitor.ts
- server/_core/polymarketAutonomy.ts
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Polymarket's /auth/api-key endpoint only accepts POST; calling it with GET returns HTTP 405 which surfaced verbatim as the connect error in the UI. Credentials derived via @polymarket/clob-client are definitionally valid at derive time, so an additional network round-trip adds no value. Replace the remote validation call with a fast format check (all three fields non-empty, ≥4 chars). Any actual auth rejection will surface on the first order attempt with a clear exchange error. Removes now-dead crypto import, buildPolymarketSignature, buildPolymarketHeaders, and POLYMARKET_CLOB_BASE_URL. https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
…er + CodeRabbit fixes
Three new free, high-edge signal sources wired into BOTH Kalshi and
Polymarket signal pipelines:
1. Awards Precursor Model (kalshiAwardsPrecursor.ts)
- SAG / DGA / PGA / BAFTA weighted predictor for Oscars
- Grammy + Emmy categories with calibrated weights
- Bayesian update: each precursor's weight is redistributed onto its
winner; remaining mass scales proportionally; sum stays = 1
- Hooks into resolveFundamentalPrior (Kalshi) + estimateFairValue
(Polymarket) so the existing value-play pipeline naturally fires
- Operator stages KNOWN_NOMINEES + KNOWN_PRECURSORS each season
2. Linguistic-Tell Detector (kalshiLinguisticTells.ts)
- Corporate code-language patterns ("strategic alternatives",
"right-sizing", "mutually agreed", …) → M&A / CEO / layoff /
earnings-miss markets
- Naive-Bayes log-odds update: each tell's empirical hit-rate
contributes a likelihood ratio relative to the prior; multiple
tells stack multiplicatively, capped at 92% to prevent overfit
- One-tell case collapses to the tell's hit-rate (sanity check
against empirical calibration)
- New SignalType "linguistic_tell" emitted by both Kalshi and
Polymarket signal generators when news snippets match
- newsSnippets threaded through MarketSentimentContext (Kalshi)
and newsSnippetsByMarket option (Polymarket)
3. Wikipedia Edit Watcher (wikipediaEditWatcher.ts)
- Polls action API every 5 min for watchlist (~10 default pages,
extensible) — within free 500-req/hour rate limit
- Detects significant edits: alarm keywords (death, indicted,
resign, scandal, lawsuit, …) OR size delta ≥ 200 bytes
- Per-page cursor prevents re-emission across consecutive polls
- Detection-only with audit-log; matches surfaced for operator
review (auto-execution gated to a future PR)
- Scheduled in server/_core/index.ts next to combinatorial-arb
scanner; wraps fetchWithRetry + a dedicated CircuitBreaker
Bonus fixes (CodeRabbit + Codex review):
- routers.ts: trim Polymarket credentials before persisting
- crossPlatformArbitrage.ts: gate prices to (0,1), reject cent-scaled
or stale "1.0" quotes
- polymarketClusterMonitor.ts: collapse redundant
`retracement.detected && retracement.detected`
- polymarketMarketMaking.ts: align minProfitPct filter with the
returned guaranteedProfitPct (ROI semantics, not discount-to-par)
- categoryPersonas.ts + grokPersonas.ts: derive id +
systemMandate from platform — Polymarket reviewers no longer
inherit Kalshi-branded copy or fee math (CTF Exchange ~2 % per leg
+ USDC/Polygon gas)
- CommandPalette / setupStatus / dashboardLanding: route
/funding → /connect, /analytics → /signals after route trim
mlSignalEnsemble: extended SIGNAL_TYPE_INDEX with new types
(linguistic_tell=8, wikipedia_edit=9), divisor bumped to 9 so
encoded values stay in [0,1].
Test coverage: 999 tests passing (up from 961).
- 14 new tests for awards precursor (probability simplex, sweep
cap, classifier, fundamental lookup)
- 14 new tests for linguistic tells (single tell, monotonicity, cap,
market-type filter, classifier)
- 7 new tests for Wikipedia watcher (alarm detection, size delta,
cursor advance, market matching, error handling)
- ml-ensemble tests updated for new divisor
https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
…wner address Two correctness fixes flagged by Codex review on PR #57. P1 — polymarketAutonomy.ts NO-side sizing: estimateSizeForRiskBudget() expects the win probability of the *purchased* token, but the autonomy loop was always passing `best.fairValueEstimate` which is the YES probability. For `best.side === "no"` candidates that reverses Kelly: a contrarian NO on a 93% YES market got sized as if NO had ~80% win probability, producing wildly oversized live orders. Flip the probability for NO before sizing. P2 — polymarketAuth.ts placement gate: When POLYMARKET_OWNER_ADDRESS is unset, syncPolymarketPositions() silently no-ops. Live orders still went through, but the exit monitor never saw the resulting positions — no auto-close, no trailing stop, no drift-close. Block live placement until the env var is set. Failing closed beats trading positions we can't reconcile. https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
…base scaffold + CodeRabbit fixes
Implements the plan in /root/.claude/plans/sparkling-churning-dusk.md.
Targets the user's stated goal of arming live trading on both prediction
platforms while tracking the bot's daily sports-pick win/loss record.
Phase 0 — CodeRabbit correctness fixes (BLOCKING)
- kalshiSignals.calculateSignalPerformance: lazy-init each SignalType so
newly added types (linguistic_tell, wikipedia_edit) no longer crash on
the non-null assertion. Regression test added.
- wikipediaEditWatcher: breaker tightened to 30s/30s (CLAUDE.md profile),
cursor advance is now monotonic (no rollback on overlapping polls),
matchSignalsToMarkets gates by category when both sides expose it
(kills "musk" → music false positives). New regression tests.
- grokPersonas: getGrokPersona preserves caller's category;
${RULES_BLOCK} placeholder removed from cached mandate so
injectVerbatimRulesBlock remains the only splice path.
- linguistic-tells test: cap assertion tightened to 0.92.
Phase 1 — Polymarket Daily Sports Play (new)
- server/_core/polymarketDailySportsPlay.ts mirrors dailySportsPlay.ts.
Reuses operator's existing tradingPreferences gates so a single
liveTradingEnabled toggle arms BOTH Kalshi and Polymarket daily plays.
Sized 2.5% of bankroll (Kalshi capital proxy until USDC.e poll lands).
Filters Polymarket sports markets via category literal OR classifier
fallback (Polymarket's category strings are messier than Kalshi's).
AI-reviewed via reviewPolymarketSignalsWithTrader; full risk-gate
stack inline (existing position, drawdown breaker, portfolio + per-
category exposure caps). Per-user mutex wraps placement.
Phase 2 — dailyPlayPicks lifecycle table
- New table tracks each daily-play pick from entry to resolution across
BOTH platforms. Unique index on (userId, platform, playType, playDate)
makes inserts idempotent at the DB level.
- server/db.daily-play-picks.ts: insertDailyPlayPick (ON CONFLICT DO
NOTHING), linkPositionToPick, closeDailyPlayPickByPosition,
closeDailyPlayPickByMarketFallback, getDailyPlayPicks,
getTodayDailyPlayPicks, rollupScoreboard.
Phase 3 — Scheduler wiring
- maybeRunPolymarketDailySportsPlay added next to the Kalshi version,
same 5-min tick + per-user dedup (in-process Set + audit-log).
Phase 4 — Close-hook side effects
- closeKalshiPosition (Kalshi live + paper) closes matching daily picks
- polymarketPositionSync drift-close updates picks with estimated
exit price (snapshot mark) — flagged in audit
- paperTrading polymarket close closes matching paper picks
- upsertPolymarketPosition closes picks on transition-to-closed
(defensive — drift-close is the primary detector)
Phase 5 — tRPC daily.* namespace
- daily.getDailyPlayScoreboard returns today's picks + 30-day rollup +
lifetime win-rate by platform (Kalshi vs Polymarket).
- daily.getTodayDailyPlay returns today's row(s) only.
Phase 6 — Daily Pick Scoreboard widget
- client/src/components/widgets/DailyPlayScoreboard.tsx: 3-row compact
card (today's picks + last 30d split + lifetime tally) with per-day
table in full mode. Mounted on Dashboard.tsx after the equity chart.
Phase 7 — env vars
- ENABLE_POLYMARKET_DAILY_SPORTS_PLAY (default true)
- POLYMARKET_DAILY_SPORTS_PLAY_HOUR_UTC (default 14)
- POLYMARKET_DAILY_SPORTS_PLAY_PCT_OF_CAPITAL (default 0.025)
- ENABLE_COINBASE_LIVE (default false), COINBASE_SANDBOX_MODE (true)
Phase 8/9 — Operator pre-arm checklist + verification doc
- docs/POLYMARKET_DAILY_PLAY_VERIFICATION.md walks through every Railway
env var, per-user trading pref, funding step, self-test check, and an
11-step paper→live smoke test sequence.
Phase 10 — Coinbase scaffolding (architectural ONLY)
- drizzle/migrations/0014_coinbase_scaffold.sql + schema exports for
coinbaseCredentials, coinbaseOrders, coinbasePositions, coinbaseCapital
- server/_core/coinbaseAuth.ts (format validation only, balance fetch
stubbed), server/_core/coinbaseExecution.ts (THROWS on every call —
inert until Phase 10 build-out lands)
- server/db.coinbase-credentials.ts mirrors Polymarket creds (AES-GCM)
- coinbase.* tRPC namespace (connect/status/disconnect)
- CoinbaseConnectPanel on Connect.tsx — operator can connect now to be
ready when instruments + signal sources are decided
Verification: 1009 tests passing (up from 1002), typecheck clean.
https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
mikelaurenzo7-collab
force-pushed
the
claude/kalshi-grok-pivot-o7XWM
branch
from
7b7c1de to
1017dc3
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
mikelaurenzo7-collab
changed the title
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
server/_core/paperTrading.ts (1)
303-461:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing audit logging for simulated Polymarket order placement.
Per coding guidelines, "every order placement and cancellation" should be audited. Even simulated orders should be logged with
simulated: trueflag for complete audit trails. This is a duplicate of the past review comment that remains unaddressed.As per coding guidelines: "Audit every significant event using
db.logAuditEvent(eventType, jsonPayload, openId)for: every order placement and cancellation…"🤖 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 `@server/_core/paperTrading.ts` around lines 303 - 461, The simulatePolymarketOrderFill function currently doesn't write an audit record for the simulated order; add a call to db.logAuditEvent(...) after the DB insert (or on success before returning) that logs the order placement with the clientOrderId, marketId, tokenId, side, sizeUsdc, limitPrice/fillPrice and include simulated: true in the JSON payload, passing _triggeredByOpenId as the openId argument; use the same clientOrderId variable and set eventType like "polymarket.order.placed" to follow existing audit naming.client/src/pages/Connect.tsx (1)
408-412:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStill missing the 4-character Polymarket client check.
The submit path and disabled state still only reject empty strings, so values like
"abc"get through even though this flow is supposed to be a pure client-side format check requiring all three L2 fields to be at least 4 characters.Also applies to: 698-705
🤖 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 `@client/src/pages/Connect.tsx` around lines 408 - 412, The connect flow currently only checks for non-empty strings in handleConnect (variables trimmedApiKey, trimmedApiSecret, trimmedApiPassphrase) and in the form's disabled state, so 1–3 char values still pass; update both the handleConnect validation and the UI disabled logic to require each L2 field to have length >= 4 (e.g., check trimmedApiKey.length >= 4, trimmedApiSecret.length >= 4, trimmedApiPassphrase.length >= 4) and surface the same error message when the check fails so the submit path and disabled state are consistent.
🤖 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 `@drizzle/migrations/0014_coinbase_scaffold.sql`:
- Around line 28-35: The migration creates coinbaseCredentials.sandboxMode as
boolean but the Drizzle model and DB writes use an integer(1/0); change the
column definition in the migration for "coinbaseCredentials" so "sandboxMode" is
an integer NOT NULL (e.g., integer NOT NULL DEFAULT 1 or 0 to match desired
sandbox default) instead of boolean to avoid type mismatch during the first
upsert; update the DEFAULT to match the model's expected 1/0 value.
In `@server/_core/dailyMoonshotPlay.ts`:
- Around line 622-623: The playDate is being computed after the AI review/order
path completes which can cross UTC midnight; compute and persist a run-anchored
date at the start of the run and use that for idempotency and lifecycle rows.
Specifically, introduce a runStartPlayDate (compute once with new
Date().toISOString().slice(0,10) or equivalent) at the beginning of the
top-level run function that orchestrates the review/order flow, replace the late
computation of playDate with this runStartPlayDate wherever the (userId,
platform, playType, playDate) idempotency key or moonshot lifecycle persistence
(the playDate variable in dailyMoonshotPlay.ts) is used, and thread/run this
value through helper functions or the context so all downstream code (AI review,
order execution, persistence) references the same anchored date. Ensure any
tests or callers that assume the previous timing are updated to use the
run-anchored date.
In `@server/_core/env.ts`:
- Around line 349-357: The new feature flag enablePolymarketDailySportsPlay is
currently defaulting to true; change its default to false so the Polymarket
daily play is opt-in by updating the normalizeBoolean call for
enablePolymarketDailySportsPlay to use false as the second argument; ensure you
only modify the default value in the enablePolymarketDailySportsPlay declaration
(the symbol name enablePolymarketDailySportsPlay) and do not alter
normalizeBoolean semantics so that on lookup failures the system still falls
back to safe (paper) behavior.
In `@server/_core/polymarketDailySportsPlay.ts`:
- Around line 66-68: The current flow submits the external Polymarket order
before reserving the daily play row, allowing duplicate buys on retry/concurrent
runs; change the flow in runPolymarketDailySportsPlay (or the top-level handler
in polymarketDailySportsPlay.ts) to acquire the Postgres distributed lock (use
the helper in distributedLock.ts) or atomically reserve the dailyPlayPicks key
for the user/day before any external side-effect, then perform the order
submission, and only after success update the dailyPlayPicks bookkeeping; ensure
the lock is released on completion/failure so only one autonomy run per user/day
can place an order.
- Around line 79-99: polymarketDailySportsPlay reads getTradingPreferences but
never enforces prefs.maxDailyOrders, allowing users who've exhausted their daily
budget elsewhere to receive an extra order; after obtaining prefs (the block
using getTradingPreferences and the prefs variable) add a check that reads the
user's current-day order count across Polymarket and autonomy activity (use or
add a helper such as
getDailyOrderCount/getUserDailyOrderCountAcrossPlatforms(userId)) and compare it
to prefs.maxDailyOrders, returning status "disabled" with an appropriate reason
when the count >= prefs.maxDailyOrders; ensure this check runs before allowing a
trade to proceed so the runner respects the shared daily limit.
In `@server/_core/polymarketSignals.ts`:
- Around line 217-223: The current guard around the awards override drops exact
0 or 1 from lookupAwardsFundamental, causing fallback to the blend; update the
condition in polymarketSignals (the block using awardsPrior and
lookupAwardsFundamental) to accept boundary probabilities by checking
awardsPrior != null && Number.isFinite(awardsPrior) && awardsPrior >= 0 &&
awardsPrior <= 1 so that exact 0 and 1 are returned directly.
In `@server/db.daily-play-picks.ts`:
- Around line 177-209: The fallback predicate is too broad in
closeDailyPlayPickByMarketFallback and may update multiple pending rows; narrow
it by requiring either a specific play identifier (add args.playDate or
args.playType) and include eq(dailyPlayPicks.playDate, args.playDate) or
eq(dailyPlayPicks.playType, args.playType) in the conditions (and keep the
existing tokenId/platform/marketId checks), and additionally use a single-row
safeguard such as calling .limit(1) on the update (if supported by your query
API) or include a unique createdAt/entryAt match (e.g.,
eq(dailyPlayPicks.createdAt, args.createdAt)) to ensure only the intended pick
is closed; update the function signature and all call sites accordingly
(references: closeDailyPlayPickByMarketFallback, dailyPlayPicks, marketId,
tokenId, platform, status).
- Around line 106-130: The linkPositionToPick function can match multiple rows
when different play types (e.g., "sports" vs "moonshot") share the same
marketId/playDate; update the function signature to accept a playType (add
playType: DailyPlayPlayType or similar to the args) and include a predicate
eq(dailyPlayPicks.playType, args.playType) in the conditions array before
executing the update so the update is scoped to the correct play type; also
update any callers to pass the playType.
---
Duplicate comments:
In `@client/src/pages/Connect.tsx`:
- Around line 408-412: The connect flow currently only checks for non-empty
strings in handleConnect (variables trimmedApiKey, trimmedApiSecret,
trimmedApiPassphrase) and in the form's disabled state, so 1–3 char values still
pass; update both the handleConnect validation and the UI disabled logic to
require each L2 field to have length >= 4 (e.g., check trimmedApiKey.length >=
4, trimmedApiSecret.length >= 4, trimmedApiPassphrase.length >= 4) and surface
the same error message when the check fails so the submit path and disabled
state are consistent.
In `@server/_core/paperTrading.ts`:
- Around line 303-461: The simulatePolymarketOrderFill function currently
doesn't write an audit record for the simulated order; add a call to
db.logAuditEvent(...) after the DB insert (or on success before returning) that
logs the order placement with the clientOrderId, marketId, tokenId, side,
sizeUsdc, limitPrice/fillPrice and include simulated: true in the JSON payload,
passing _triggeredByOpenId as the openId argument; use the same clientOrderId
variable and set eventType like "polymarket.order.placed" to follow existing
audit naming.
🪄 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: 2d95acba-6948-434a-9a9b-13b147a6e145
📒 Files selected for processing (44)
client/src/components/CommandPalette.tsxclient/src/components/widgets/DailyPlayScoreboard.tsxclient/src/lib/dashboardLanding.test.tsclient/src/lib/dashboardLanding.tsclient/src/lib/setupStatus.tsclient/src/pages/Connect.tsxclient/src/pages/Dashboard.tsxdocs/POLYMARKET_DAILY_PLAY_VERIFICATION.mddrizzle/migrations/0013_daily_play_picks.sqldrizzle/migrations/0014_coinbase_scaffold.sqldrizzle/schema.tsserver/_core/categoryPersonas.tsserver/_core/coinbaseAuth.tsserver/_core/coinbaseExecution.tsserver/_core/crossPlatformArbitrage.tsserver/_core/dailyMoonshotPlay.tsserver/_core/dailySportsPlay.tsserver/_core/env.tsserver/_core/grokPersonas.tsserver/_core/index.tsserver/_core/kalshiAwardsPrecursor.tsserver/_core/kalshiLinguisticTells.tsserver/_core/kalshiSignals.tsserver/_core/mlSignalEnsemble.tsserver/_core/paperTrading.tsserver/_core/polymarketAuth.tsserver/_core/polymarketAutonomy.tsserver/_core/polymarketClusterMonitor.tsserver/_core/polymarketDailySportsPlay.tsserver/_core/polymarketMarketMaking.tsserver/_core/polymarketPositionSync.tsserver/_core/polymarketSignals.tsserver/_core/wikipediaEditWatcher.tsserver/daily-play-picks.test.tsserver/db.coinbase-credentials.tsserver/db.daily-play-picks.tsserver/db.polymarket.tsserver/db.tsserver/kalshi.awards-precursor.test.tsserver/kalshi.linguistic-tells.test.tsserver/kalshi.signals.test.tsserver/ml-ensemble.test.tsserver/routers.tsserver/wikipedia-edit-watcher.test.ts
✅ Files skipped from review due to trivial changes (6)
- client/src/lib/dashboardLanding.test.ts
- client/src/lib/dashboardLanding.ts
- server/kalshi.awards-precursor.test.ts
- server/wikipedia-edit-watcher.test.ts
- server/kalshi.linguistic-tells.test.ts
- server/_core/polymarketClusterMonitor.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- server/_core/grokPersonas.ts
- server/_core/mlSignalEnsemble.ts
- server/_core/kalshiSignals.ts
- server/_core/wikipediaEditWatcher.ts
- server/_core/polymarketAutonomy.ts
- server/_core/kalshiLinguisticTells.ts
- server/_core/polymarketPositionSync.ts
- client/src/components/CommandPalette.tsx
- server/_core/crossPlatformArbitrage.ts
- server/_core/kalshiAwardsPrecursor.ts
- server/_core/categoryPersonas.ts
- client/src/lib/setupStatus.ts
Comment on lines
+349
to
+357
| // ── Polymarket Daily Sports Play (mirrors Kalshi version, ON by default) ─ | ||
| // Once per UTC day at the configured hour, fires the top-EV reviewed | ||
| // sports signal on Polymarket sized at 2.5% of bankroll. Reuses the | ||
| // operator's `tradingPreferences.{liveTradingEnabled, autonomyMode, | ||
| // executionCadence}` gates — same kill-switch arms BOTH platforms. | ||
| enablePolymarketDailySportsPlay: normalizeBoolean( | ||
| process.env.ENABLE_POLYMARKET_DAILY_SPORTS_PLAY, | ||
| true, | ||
| ), |
There was a problem hiding this comment.
Make the new Polymarket daily play opt-in.
Defaulting ENABLE_POLYMARKET_DAILY_SPORTS_PLAY to true means an already-armed live deployment with saved Polymarket credentials can start placing a brand-new class of real orders immediately after upgrade. This should ship fail-closed and require an explicit enable.
🔒 Proposed fix
enablePolymarketDailySportsPlay: normalizeBoolean(
process.env.ENABLE_POLYMARKET_DAILY_SPORTS_PLAY,
- true,
+ false,
),As per coding guidelines, On lookup failure (DB outage), return true (paper) — never silently allow live trading on error.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ── Polymarket Daily Sports Play (mirrors Kalshi version, ON by default) ─ | |
| // Once per UTC day at the configured hour, fires the top-EV reviewed | |
| // sports signal on Polymarket sized at 2.5% of bankroll. Reuses the | |
| // operator's `tradingPreferences.{liveTradingEnabled, autonomyMode, | |
| // executionCadence}` gates — same kill-switch arms BOTH platforms. | |
| enablePolymarketDailySportsPlay: normalizeBoolean( | |
| process.env.ENABLE_POLYMARKET_DAILY_SPORTS_PLAY, | |
| true, | |
| ), | |
| // ── Polymarket Daily Sports Play (mirrors Kalshi version, ON by default) ─ | |
| // Once per UTC day at the configured hour, fires the top-EV reviewed | |
| // sports signal on Polymarket sized at 2.5% of bankroll. Reuses the | |
| // operator's `tradingPreferences.{liveTradingEnabled, autonomyMode, | |
| // executionCadence}` gates — same kill-switch arms BOTH platforms. | |
| enablePolymarketDailySportsPlay: normalizeBoolean( | |
| process.env.ENABLE_POLYMARKET_DAILY_SPORTS_PLAY, | |
| false, | |
| ), |
🤖 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 `@server/_core/env.ts` around lines 349 - 357, The new feature flag
enablePolymarketDailySportsPlay is currently defaulting to true; change its
default to false so the Polymarket daily play is opt-in by updating the
normalizeBoolean call for enablePolymarketDailySportsPlay to use false as the
second argument; ensure you only modify the default value in the
enablePolymarketDailySportsPlay declaration (the symbol name
enablePolymarketDailySportsPlay) and do not alter normalizeBoolean semantics so
that on lookup failures the system still falls back to safe (paper) behavior.
- migration 0014: sandboxMode boolean → integer to match Drizzle model - polymarketSignals: accept boundary 0/1 from awards prior (was dropping exact-0 / exact-1 outputs and falling back to naive blend) - linkPositionToPick: require playType so sports/moonshot picks on the same market don't collide on linkage - closeDailyPlayPickByMarketFallback: require playDate (and accept optional playType) so the fallback sweep doesn't close unrelated pending rows; all 4 call sites pass today's UTC date - dailySportsPlay / dailyMoonshotPlay / polymarketDailySportsPlay: anchor playDate at run-start so a long AI review crossing UTC midnight doesn't drift the idempotency key - polymarketDailySportsPlay: enforce prefs.maxDailyOrders against getTodayPolymarketOrderCount (new helper) before placing the order - polymarketDailySportsPlay: reserve dailyPlayPicks row BEFORE order placement (idempotent gate via the unique index); on order failure, void the reservation instead of leaving it pending - voidDailyPlayPick: new helper for the failed-reservation path - paperTrading.simulatePolymarketOrderFill: emit polymarket_order_placed audit event with simulated:true flag (per CLAUDE.md "audit every order placement") - Connect.tsx: enforce ≥4-char min on all 3 Polymarket L2 fields in both the submit handler and the disabled-button state (matches the server-side format guard in validatePolymarketCredentials) 1009 tests pass, typecheck clean. Skipped CodeRabbit's request to default ENABLE_POLYMARKET_DAILY_SPORTS_PLAY to false — the operator explicitly requested true-by-default during plan approval; their env file already sets it explicitly so the default doesn't affect this deployment. https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/_core/dailyMoonshotPlay.ts (1)
95-127:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEmit moonshot run start/skipped audit events across early-return paths.
Line 100 onward has multiple early exits (
disabled, preference gates, credential/balance failures) without autonomy-run lifecycle audit entries. That leaves run-level observability and compliance incomplete.Suggested pattern
+ const auditActor = `user:${userId}`; + const auditRun = (eventType: string, payload: Record<string, unknown>) => + logAuditEvent(eventType, JSON.stringify(payload), auditActor).catch(() => {}); + + await auditRun("kalshi_daily_moonshot_play_start", { + userId, + playDate: runPlayDate, + }); + if (!ENV.enableDailyMoonshot) { + await auditRun("kalshi_daily_moonshot_play_skipped", { + userId, + reason: "ENABLE_DAILY_MOONSHOT is not set", + }); return { status: "disabled", reason: "ENABLE_DAILY_MOONSHOT is not set", }; }As per coding guidelines,
server/**/*.ts: "Audit every significant event usingdb.logAuditEvent(...)for ... every autonomy run (start, skipped, executed, error)."🤖 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 `@server/_core/dailyMoonshotPlay.ts` around lines 95 - 127, The early-return branches in dailyMoonshotPlay (the ENV.enableDailyMoonshot check and the trading-preferences gates around getTradingPreferences, plus other credential/balance failures) currently return without emitting run-level audit entries; update dailyMoonshotPlay to call db.logAuditEvent for run lifecycle events: log a "run_started" audit right after computing runStartedAt/runPlayDate (including userId, platform, playType, runPlayDate, runStartedAt) and log a matching "run_skipped" audit before each early return (include a clear reason string and the same identifiers), and ensure the same pattern is applied for any credential/balance failure branches so every autonomy run path emits start and skipped/executed/error events via db.logAuditEvent.
🤖 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 `@server/db.polymarket.ts`:
- Around line 182-187: The fallback call to closeDailyPlayPickByMarketFallback
is using the current UTC date (new Date().toISOString().slice(0,10)) as playDate
which can fail to match picks opened on an earlier UTC day; change playDate to
the pick's entry/run day derived from the position (e.g., convert the position's
entry timestamp field such as position.entryTime / position.entryAt /
position.entryDate to an ISO date string slice(0,10)) and pass that instead,
keeping the other parameters (scopedUserId, position.marketId, position.tokenId)
unchanged so the fallback keys the same day the pick was opened.
---
Outside diff comments:
In `@server/_core/dailyMoonshotPlay.ts`:
- Around line 95-127: The early-return branches in dailyMoonshotPlay (the
ENV.enableDailyMoonshot check and the trading-preferences gates around
getTradingPreferences, plus other credential/balance failures) currently return
without emitting run-level audit entries; update dailyMoonshotPlay to call
db.logAuditEvent for run lifecycle events: log a "run_started" audit right after
computing runStartedAt/runPlayDate (including userId, platform, playType,
runPlayDate, runStartedAt) and log a matching "run_skipped" audit before each
early return (include a clear reason string and the same identifiers), and
ensure the same pattern is applied for any credential/balance failure branches
so every autonomy run path emits start and skipped/executed/error events via
db.logAuditEvent.
🪄 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: 9ccc1cda-8b1e-4234-a22e-aa140240e0ed
📒 Files selected for processing (11)
client/src/pages/Connect.tsxdrizzle/migrations/0014_coinbase_scaffold.sqlserver/_core/dailyMoonshotPlay.tsserver/_core/dailySportsPlay.tsserver/_core/paperTrading.tsserver/_core/polymarketDailySportsPlay.tsserver/_core/polymarketPositionSync.tsserver/_core/polymarketSignals.tsserver/db.daily-play-picks.tsserver/db.polymarket.tsserver/db.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- server/_core/dailySportsPlay.ts
- server/db.ts
- server/_core/polymarketSignals.ts
- client/src/pages/Connect.tsx
- server/_core/polymarketPositionSync.ts
- drizzle/migrations/0014_coinbase_scaffold.sql
- server/db.daily-play-picks.ts
- server/_core/polymarketDailySportsPlay.ts
…kipped audit events - db.polymarket.ts: closeDailyPlayPickByMarketFallback now keys by existing.openedAt instead of current UTC date — prevents unmatched fallback when a pick was opened on an earlier UTC day and linking failed - dailyMoonshotPlay.ts: emit kalshi_daily_moonshot_play_skipped audit event before each meaningful early return (prefs gates, credentials missing, balance unknown) per CLAUDE.md mandate to audit every autonomy run lifecycle event https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Saw your post about feat: arm live trading + daily sports scoreboard +. I build MT5 trading bots — entry/exit logic, risk management, live broker integration. Running within 48h. Price: $500. I'll send a full spec before you commit. — Lyn Reply or email: lynchatta@gmail.com |
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
3 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.
Summary
validatePolymarketCredentialswas callingGET /auth/api-keywhich returns HTTP 405 — replaced with format-only validation (credentials are checked for real on first order attempt)side === "no", the purchased-token probability must be1 - fairValueEstimate(YES probability), notfairValueEstimate— was severely mis-sizing NO positionsplacePolymarketOrdernow hard-blocks ifPOLYMARKET_OWNER_ADDRESSis unset, preventing silent position-sync blind spotsresolveFundamentalPrior(Kalshi) andestimateFairValue(Polymarket)log(p/(1-p)) − priorLogOdds), caps at ±0.92calculateSignalPerformancelazy-init (was crashing on newSignalTypes);grokPersonascategory tag propagation;${RULES_BLOCK}placeholder removed from cached mandate;crossPlatformArbitrageprice gates;polymarketClusterMonitorredundant boolean;polymarketMarketMakingguaranteed-profit calculation orderdailySportsPlay.tsfor Polymarket — fires at 14:30 UTC by default (POLYMARKET_DAILY_SPORTS_PLAY_HOUR_UTC), gated byENABLE_POLYMARKET_DAILY_SPORTS_PLAY=truedailyPlayPickstable + DB helpers: tracks bot's win/loss record across both platforms; unique on(userId, platform, playType, playDate); status lifecycle:pending → won/lost/closed_breakeven/partial; close hooks wired into all Kalshi + Polymarket close pathscoinbase.*namespace, Connect page panel —placeCoinbaseOrderthrows on every call until instruments + signals + risk caps are decideddailyPlayPickstable + enums + indexesTest plan
corepack pnpm test -- --run)corepack pnpm check)connecteddailyPlayPicksrow inserted withstatus='pending'dailyPlayPicksstatus flips towon/lostwith exit price + realized PnLENABLE_POLYMARKET_DAILY_SPORTS_PLAY=falseno Polymarket daily play scheduler runsSee
docs/POLYMARKET_DAILY_PLAY_VERIFICATION.mdfor the full 11-step operator smoke-test sequence.https://claude.ai/code/session_01To82bdNQsUtCeAyGJFLQc3
Summary by CodeRabbit
New Features
Improvements
Navigation & UI
Configuration