(feat) apply improvements requested by ralph - #214
Merged
Conversation
# Conflicts: # frontend/src/pages/Dex.tsx # frontend/src/pages/DexPool.tsx
Greptile SummaryThis PR improves DEX workspace controls and executor configuration while synchronizing each user's default server between the dashboard and Telegram.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue identified. The changed default-server, DEX workspace, and executor paths preserve their relevant server, pool, and interval contracts under the reachable scenarios reviewed.
|
| Filename | Overview |
|---|---|
| condor/web/routes/servers.py | Exposes the user's effective default server alongside concurrently resolved server status. |
| condor/web/routes/settings.py | Adds effective default-server metadata to the settings server response. |
| frontend/src/components/layout/ServerSelector.tsx | Seeds an empty browser selection from the online default and marks it in the menu. |
| frontend/src/components/settings/ServersSettings.tsx | Displays default state and synchronizes a successful default change with the active browser server. |
| frontend/src/components/dex/PoolSourceTabs.tsx | Moves chain and venue controls into reusable anchored menus. |
| frontend/src/components/executor/lp-config.ts | Calculates Meteora DLMM bin span and warns when a configured range exceeds the position cap. |
| frontend/src/pages/DexPool.tsx | Refines pool identity, chart intervals, and locked-pool executor configuration. |
| tests/test_web_servers_parallel_status.py | Updates response assertions and verifies default-server flag selection. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
CM[Config manager chat default] --> API[Server list APIs]
API --> SS[Server settings]
API --> SEL[Server selector]
SS -->|Set default| CM
SS -->|Activate server| SEL
DP[DEX pool workspace] --> OC[Spot order configuration]
DP --> LPC[Locked LP configuration]
DP --> CH[Pool chart and statistics]
Reviews (1): Last reviewed commit: "(feat) apply improvements requested by r..." | Re-trigger Greptile
…Address TradeChart hand-rebuilt the candle channel key without the pool segment in its live-tick listener and setData signature, so pool-pinned charts subscribed to a channel that never notifies and froze after the REST backfill. Extract a canonical candleChannelKey() in candle-store.ts and use it in useCandleStore and both TradeChart effects, with poolAddress in the dependency arrays. CLOB keys stay byte-identical (pool segment appended only when truthy).
… good cache fetch_current_price, fetch_tickers and fetch_ticker_pool swallowed every exception and returned empty shapes, so a transient timeout/500 during an SDS poll was cached as a fresh success: last good TICKER_POOL/PRICES clobbered, consecutive_errors reset, health.record_success during an outage, and a null frame broadcast to prices WS subscribers. Add strict=False params that re-raise on failure (404 'endpoint absent' stays a real empty answer) and register them strict in register_default_fetches, matching the existing strict-fetcher pattern. Direct callers keep the lenient default. Closes CORR-188 (backend).
…193) BotLedger._save drops the fixed-temp-name tmp+replace dance for atomic_write_json; config.yml, learnings.md, AGENT.md, strategy.md and journal templates switch from plain write_text to atomic_write_text. Append-only and write-once files (snapshots, experiments) unchanged.
sessions.py hand-rolled parse_custom_agent_key + find_custom_provider + CUSTOM_LLM_* env fallbacks, duplicating the shared resolver and losing its user_id fallback. Add strict=True to resolve_custom_endpoint so the session path keeps its actionable RuntimeError for a missing named endpoint, and call the one resolver from sessions.py.
/agents/notify, /delegate, /consult and the strategy start routes forwarded a body-supplied chat_id to outbound Telegram sends with no ownership check, so any approved user could speak with the bot's identity into anyone's chat. _check_chat_access now allows the caller's own private chat for free, verifies group membership against Telegram through the same bot ladder that delivers (_HttpBot learns getChatMember), fails closed with 403, and exempts admins (mirroring SEC-081/SEC-152).
Web login moved to one-time tokens from /web long ago; the widget path (verify_telegram_login, _AUTH_WINDOW_SECONDS, LoginRequest) was fully unreferenced yet looked like a live HMAC login path during security review. Also drops the now-unused hashlib/hmac/TELEGRAM_TOKEN imports.
- stopMutation now invalidates [executors, server] (prefix covers main/altPair variants) and [dex-lp-executors, server] so stopped executors leave the pane within one refetch cycle; keeps [executors-infinite, server] for the Executors page - drop dead [positions, server] invalidation (no useQuery ever reads it); [consolidated-positions, server] is the real key
One shared components/dex/format.ts replaces the three divergent private copies in PoolBrowser, PoolStats and lp-position. PoolStats' 24h-change and APR cells now use pct() with the K% cap, matching the table, and a genuine $0 renders as $0.00 instead of an em-dash (dash is only for missing data).
…iceSig (ARCH-194)
… on leftover search text (CORR-185)
…config_path, _sum_snapshot_value, get_session_dir (READ-203)
…ads (SEC-197)
GET /settings/gateway/networks/{id} shipped the full network config to any
trader-level seat, and paid RPC providers embed the API key in the nodeURL
(Helius query string, Infura/Alchemy path, or userinfo). The route now masks
every secret-bearing URL part server-side for callers below the OWNER line —
keeping scheme://host visible so a trader still sees which endpoint they trade
against — with the same admin bypass as _require_owner, and reports a
redacted flag. The frontend renders the Networks & RPC editor read-only for
non-owners (OWNER_ONLY_HINT) instead of a form whose save is refused, so a
masked value can never round-trip into a save; owners and admins keep the
full edit flow, and POST stays owner-gated (SEC-166).
fetch_ohlcv and fetch_liquidity_bins no longer take user_data/chat_id/context: a pool's candles and bins are global answers, so both now cache process-wide in pool_data's own TTL caches, keyed on the upstream arguments. fetch_ohlcv gains use_cache=False (the live poll's freshness bypass, previously spelled user_data=None); fetch_liquidity_bins requires an already-resolved client, so client resolution moves to the callers like every fetchers/ module. The '_cache'-namespace stay-in-step contract with handlers/dex/_shared is gone, and dex_candles drops its module-dict-as-user_data workaround. (ARCH-194)
… (ARCH-190) condor/ core no longer imports from handlers/: the platform-neutral pieces of handlers/agents/_shared.py moved down — MCP toolset builders to condor/runtime/toolsets.py, danger classification + confirmation summary to condor/runtime/danger.py, context builders to condor/runtime/context.py, the model catalog to condor/llm/options.py, TelegramChannel to condor/runtime/channels.py — and readiness/openrouter_models/custom_models moved wholesale to condor/llm/. clean_config_for_save moved to condor/controller_configs.py. handlers modules re-export (or sys.modules- alias) the moved names so Telegram callers are unchanged; the lazy function-local imports across condor/runtime, condor/agents and condor/web are now normal top-level imports, and the trade-gate danger lists no longer live in a hot-reloaded handlers module.
…or/agents/attribution The ownership-window tiling that attributes each bot's realized PnL/volume/trades/fees across the sessions that operated it lived inside web/routes/agents.py, hand-synchronized with a parallel merge in agents/performance.py. Both now consume one implementation: condor.agents.attribution owns session ownership resolution, owner-window tiling, the slice-and-merge fold (fees_known heuristic) and the cum_fees_quote fallback, plus the current-owner rule for the live open book. Routes keep only HTTP concerns and the TTL caches. A parity test pins that the web rollup and fetch_agent_performance report identical figures for a session that adopted a long-running bot. (ARCH-191)
…ngbot stream families into condor/web/streams
|
Too many files changed for review (194 files, 100 file limit). Bypass the limit by tagging |
…iled saves and replace ticker collisions in place
…'s Start/End/Limit
Contributor
|
Commit fec6958 Default server, shared with Telegram
DEX pool workspace ✅
Order panel ✅
Meteora bin-count warning ✅
Empty-state fix ✅
Commit 2340c08 LP chart line labels
Gateway token recovery banner
|
Contributor
|
Commit 84a2531 Default server shared with Telegram
Gateway token listing ✅
DEX pool position scope ✅
|
rapcmia
approved these changes
Aug 20, 2026
…h and the four producers Everything Condor said when a background task finished went to Telegram and nowhere else, so a user with only the dashboard open learned nothing. condor/notifications.py is the other half: a per-user JSON store under data/ (atomic, capped, newest first, serialised behind a write lock) plus a push bus that surfaces register themselves on, the same shape runtime/wake.py already uses. chat_ws registers a sink emitting a `notification` event on the socket every authenticated page already holds; GET/POST /api/v1/notifications is the durable path for a notice raised while no tab was open. The four producers -- a finished delegation, a finished routine, /agents/notify and the boot notice -- record alongside their existing Telegram push, with the same text, so the surfaces cannot tell different stories about one task. A routine with no conversation behind it (the scheduler, the dashboard) now reaches its owner for the first time. NotifyBot is the last rung of resolve_bot(): with no TELEGRAM_TOKEN at all an outbound message becomes a bell item instead of being dropped by a sender that cannot deliver. It sits below both Telegram rungs, so a configured install is unchanged and nothing is ever pushed to Telegram twice. FEAT-048
A bell next to the Bug and Settings buttons, with an unread badge and a dropdown of the last 50 notices; an item carrying a link navigates there, and opening the list marks it read. Two sources, one react-query key: history from GET /notifications on mount, live arrivals written into the same cache by the chat socket's `notification` handler. So a notice that arrived while the tab was open and one that arrived an hour ago are indistinguishable, and a socket that was down costs nothing but latency. FEAT-048
… and /auth/local-login Telegram was both the runtime dependency and the identity provider: main() could not build an Application without a token, and the dashboard's only way to get a session was the one-time token /web mints. CONDOR_MODE=local now boots the process with no bot at all and logs the local admin straight in. The mode is explicit and is never inferred from whether a token happens to be present — telegram mode with an empty TELEGRAM_TOKEN exits at boot naming `make setup`, so an install that loses its token stops instead of quietly becoming a login-less dashboard. Local mode binds 127.0.0.1 only, with WEB_HOST as the one documented opt-out. Those two controls are independent and both required; tests pin each of them. Boot skips the PTB lifecycle rather than stubbing it: the Application is built on a placeholder token but never initialized, so nothing polls and no handler can dispatch, and the job queue is started directly (scheduled routines, update checks and signals all keep working). Outbound messages resolve to NotifyBot, so a routine's context.bot.send_message lands on the dashboard bell. Auth adds one endpoint. get_current_user, the WS handshake, check_server_access and the 200-odd guarded endpoints are untouched — /auth/local-login mints an ordinary JWT and 404s outside local mode, so nothing downstream ever learns which mode it is in.
Setup opens with the choice — Telegram (today's prompts, verbatim, plus an explicit CONDOR_MODE=telegram) or Local, which asks nothing: it writes CONDOR_MODE=local, ADMIN_USER_ID=1 and the localhost WEB_URL, and prints what local mode costs. An install from before this existed has a token but no CONDOR_MODE; the next run records telegram for it, so the mode is always explicit on disk. The Login page asks the server how this install authenticates before it renders, so the "run /web in Telegram" card never flashes on a machine that has no Telegram, and local mode lands straight on the dashboard. README documents local mode as what it is: no login, loopback only, and WEB_HOST=0.0.0.0 as a deliberate act with a warning attached.
Contributor
|
Commit ac0544b Test default server shared with Telegram ✅
Test make install ✅
Test CONDOR_MODE={local, telegram}
|
… boot Ralph's report on #214: an install configured for Telegram that flips CONDOR_MODE=local boots fine, auto-logs-in, and 500s in the browser with "user 1 is not configured in config.yml — re-run `make setup`" — and the re-run then prints "already configured" and changes nothing. Four causes, all of them "two things that must agree, and nothing binding them": * Local mode had its own identity knob. ADMIN_USER_ID decides who exists in config.yml; CONDOR_LOCAL_USER_ID decided who the dashboard logs in as, and they only coincided because `make setup` happened to write 1 to both. The knob is gone: local mode logs in as ADMIN_USER_ID, defaulting to 1 for an install that never had a Telegram id. A Telegram install that switches now logs in as *you*, keeping its servers, preferences and defaults. It also answers the question the wizard could not: 1 is not an identity, it is the admin id you do not have yet. * A non-numeric ADMIN_USER_ID was swallowed, leaving an install with no admin at all — no admin panel, no approvals, no boot notification, no local login — each surfacing later as unrelated breakage. It is now a boot error naming the value. * Local mode's user is checked at boot (check_local_user), where the .env is, instead of by the login endpoint with the browser already open. * `make setup` could not repair or switch a configured install: it printed "already configured" and skipped the block, so the advice every one of those errors gives led nowhere. It now shows the current mode, offers the other one, and stops clobbering a usable ADMIN_USER_ID with 1. It also reads the mode with a parser instead of `source`, which executes .env — one value with shell metacharacters (an API key with parentheses will do) is a syntax error that silently drops every variable below it, including a hand-added CONDOR_MODE=local, which the legacy-install migration then rewrote back to telegram. `make run` now prints the pane on a startup failure instead of "run make run-fg to see the error", so these land where they are read.
Contributor
Author
|
Thanks @rapcmia — the telegram→local report was exactly right, and it turned out to have four causes, not one. All fixed in 1bd487b. 1. Local mode had two identities for one user
is no longer needed — there is no second variable to keep in sync. Verified end-to-end against a Your servers, permissions and defaults all come with you, because they were always keyed on that same id. And it answers the "why 1, can I use 2?" question: 2.
|
… position attribution These cover work already on this branch but were never staged: the config.yml -first default server (and the pinned-server case the inversion must not break), the GeckoTerminal refusal counters and the 503-vs-404 the pool lookup answers when it was throttled, and the executor_ids a held position carries so a pool page can tell its own hold from another pool's.
…oard Two halves of the same hole. `install` was only ever emitted from the Telegram consent callback, so an install whose admin never tapped the prompt was never counted at all — and a local-mode install, which has no bot to be prompted through, could not be counted even in principle. Its consent stayed `unknown` forever, pinned to the ping floor with no way to say otherwise. Counting now happens in `telemetry.init()`, at the ping floor, without an answer, guarded by `consent.mark_install_reported()` so it stays exactly once per install whichever surface gets there first. That makes the claim the module docstring already made — "every install is counted, from the first boot, with no answer required" — true rather than aspirational. Asking now happens in the dashboard too: a one-question banner in the shell for an install that has never answered, and a permanent Settings → Privacy card that every seat can read and only the admin can change. Both render the same `DISCLOSURE` the Telegram message is built from, served by `GET /settings/telemetry`, because a privacy claim written down twice is one that will eventually disagree with itself. The Telegram copy is byte-identical to before. The envelope also carries `config.mode` now (`telegram` | `local`), which is how many installs run without Telegram — the question local mode exists to answer, and previously invisible.
fengtality
added a commit
that referenced
this pull request
Aug 21, 2026
Every agents/ change this branch carried on top of ralph's branch is lifted out and re-applied on docs/lp-rebalancer-controller-guidance, which is based on this one. agents/ here is now byte-identical to feat/ralph_improvemnts, so this PR's diff no longer claims the backpack_mm and brigado deletions — those are cardosofede's in 1cbc5dc and land with PR #214. Four of the seven commits involved touched agents/ and other paths at once, so the split is by path rather than by commit; the original commits stay in this branch's history and net to zero here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fengtality
added a commit
that referenced
this pull request
Aug 21, 2026
Retargets this PR onto main now that feat/ralph_improvemnts is merged there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fengtality
added a commit
that referenced
this pull request
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Follow-up pass on the DEX trade panel work from #203, applying the review feedback from Ralph. Three themes: the default server is now shared between Telegram and the dashboard, the DEX pool workspace stops offering choices it can't honour, and the LP form warns about Meteora's bin cap before Gateway rejects the open.
Default server, shared with Telegram
The Settings star was decorative: clicking it wrote
chat_defaultsbut nothing in the UI ever reflected it, and the dashboard always auto-selected the first online server regardless.ServerInfogainsis_default, populated in both/serversand/settings/serversfromcm.get_chat_default_server(user.id)— the same entry Telegram reads, so a default set on either surface is honoured by the other.ServerSelectorseeds a fresh browser from that default (falling back to the first online server) and marks it with a star in the dropdown.ServersSettingsfills the star, shows aDefaultbadge, disables the button on the row that is already default, spins while the mutation is in flight, and now also points this browser at the server it just made default — without that second half the click had no visible effect at all.DEX pool workspace
PoolStatswas split:PoolAddress(truncated address + copy + open-on-DEX) sits in the header beside the pair, and the network moved into the stat row.poolUrlnow covers PumpSwap (viapump.fun/coin/<base>), Uniswap and PancakeSwap, with the GeckoTerminal chain-id → DEX-slug mappings those apps route on. GeckoTerminal stays the fallback.1m/5m/15m.1h/4h/1dcandles read as duplicates of the1h/1dlookback buttons next to them, and a 3-day window of 15m candles already fits GeckoTerminal's 1000-candle cap.LPConfigPaneltakeslockedPoolAddressand just states the address instead of showing resolution status, an editable address field and a provider picker that would second-guess a decision already made.PoolSourceTabsrender through the sharedAnchoredMenuinstead of each hand-rollingabsolutepositioning plus its own outside-click/Escape listeners.Order panel
SelectFieldgainsdisabled, used on the execution-strategy select when a DEX offers only market: a one-option dropdown reads as broken unless it says so.Meteora bin-count warning
A DLMM position spans at most 69 bins; a wider range failed at open time with the only trace in the Gateway log.
dlmmBinSpan(lower, upper, binStep)computes the span from the geometric bin step, returningnullfor non-DLMM pools (only those report a bin step).rangeWarningstakes the bin step and warns when the range exceedsDLMM_MAX_BINS. It's a warning, not a block — Gateway is the authority and its cap can move.DexPoolfeeds itdepth?.bin_stepfrom Gateway's bins call; the create page passes nothing, since resolve-by-pair doesn't report one.Empty-state fix
On
/dex, a pasted address that resolved to a pool is the result — the empty token-search table underneath it stays hidden rather than saying "No pools found" directly below a pool row.Tests
tests/test_web_servers_parallel_status.pycovers the new flag: exactly one row carriesis_defaultwhen a chat default is set, and nothing is flagged when there isn't one. Existing response-shape assertions updated.