fix: recognize spot-only tokens in IsToken #25

Merged
thuanle merged 4 commits from fix/issue-24-is-token-spot-only into main 2026-04-27 05:00:41 +07:00
Owner

Summary

  • Update IsToken to classify tokens via Token2SpotSymbols so spot-only tokens are accepted by the chat token gate.
  • Keep Alpha token fallback unchanged.
  • Add regression coverage for spot-only token fallback in TestIsToken.

Why

OnChatHandler routes token messages only when binancex.IsToken(text) returns true.
Previously, IsToken only checked futures-based resolution, so spot-only tokens were ignored before reaching downstream logic that already supports spot-only data.

Changes

  • internal/helper/binancex/symbol.go
    • IsToken now checks len(Token2SpotSymbols(s)) > 0 instead of Token2FutureSymbols.
  • internal/helper/binancex/resolver_test.go
    • Add test case: spot only token fallback expecting IsToken("abc") == true when ABCUSDT exists in spot pairs.

Test Plan

  • go test ./internal/helper/binancex -run TestIsToken -v
  • go test ./internal/helper/binancex -run Token2SpotSymbols_SpotOnlyFallback -v
  • go test ./internal/services/tele/commands -run SpotOnlyReachable -v
  • go test ./...

🤖 Generated with Claude Code

## Summary - Update `IsToken` to classify tokens via `Token2SpotSymbols` so spot-only tokens are accepted by the chat token gate. - Keep Alpha token fallback unchanged. - Add regression coverage for spot-only token fallback in `TestIsToken`. ## Why `OnChatHandler` routes token messages only when `binancex.IsToken(text)` returns true. Previously, `IsToken` only checked futures-based resolution, so spot-only tokens were ignored before reaching downstream logic that already supports spot-only data. ## Changes - `internal/helper/binancex/symbol.go` - `IsToken` now checks `len(Token2SpotSymbols(s)) > 0` instead of `Token2FutureSymbols`. - `internal/helper/binancex/resolver_test.go` - Add test case: `spot only token fallback` expecting `IsToken("abc") == true` when `ABCUSDT` exists in spot pairs. ## Test Plan - [x] `go test ./internal/helper/binancex -run TestIsToken -v` - [x] `go test ./internal/helper/binancex -run Token2SpotSymbols_SpotOnlyFallback -v` - [x] `go test ./internal/services/tele/commands -run SpotOnlyReachable -v` - [x] `go test ./...` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
claudecode added 1 commit 2026-04-27 04:00:08 +07:00
Gate token detection via spot symbol resolution so chat flow accepts spot-only tokens, and add regression coverage for the fallback path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thuanle requested review from codex 2026-04-27 04:02:31 +07:00
Owner

[codex] review results for PR #25.

FAIL

  1. [Medium] internal/helper/binancex/symbol.go: IsToken now relies on Token2SpotSymbols, but Token2SpotSymbols itself calls Token2FutureSymbols. That makes the API semantics confusing: a spot resolver should not implicitly be the future+spot resolver.

Suggested direction:

  • Keep Token2FutureSymbols(token) as the futures-only resolver.
  • Make Token2SpotSymbols(token) resolve spot symbols independently via data.Market.IsSpotPair, without calling Token2FutureSymbols.
  • Update IsToken to check both resolvers explicitly:
func IsToken(s string) bool {
	if len(Token2FutureSymbols(s)) > 0 {
		return true
	}
	if len(Token2SpotSymbols(s)) > 0 {
		return true
	}

	s = strings.ToUpper(s)
	return data.Market.IsAlphaToken(s)
}

If the code still needs to derive spot symbols from futures for rich-token display, please move that into a clearly named helper such as FutureSymbols2SpotSymbols or Token2RelatedSpotSymbols, rather than hiding it inside Token2SpotSymbols.

Validation run:

  • go test ./internal/helper/binancex -run TestIsToken -v
  • go test ./internal/helper/binancex -run Token2SpotSymbols_SpotOnlyFallback -v
  • go test ./internal/services/tele/commands -run SpotOnlyReachable -v
  • go test ./...
[codex] review results for PR #25. FAIL 1. [Medium] `internal/helper/binancex/symbol.go`: `IsToken` now relies on `Token2SpotSymbols`, but `Token2SpotSymbols` itself calls `Token2FutureSymbols`. That makes the API semantics confusing: a spot resolver should not implicitly be the future+spot resolver. Suggested direction: - Keep `Token2FutureSymbols(token)` as the futures-only resolver. - Make `Token2SpotSymbols(token)` resolve spot symbols independently via `data.Market.IsSpotPair`, without calling `Token2FutureSymbols`. - Update `IsToken` to check both resolvers explicitly: ```go func IsToken(s string) bool { if len(Token2FutureSymbols(s)) > 0 { return true } if len(Token2SpotSymbols(s)) > 0 { return true } s = strings.ToUpper(s) return data.Market.IsAlphaToken(s) } ``` If the code still needs to derive spot symbols from futures for rich-token display, please move that into a clearly named helper such as `FutureSymbols2SpotSymbols` or `Token2RelatedSpotSymbols`, rather than hiding it inside `Token2SpotSymbols`. Validation run: - `go test ./internal/helper/binancex -run TestIsToken -v` - `go test ./internal/helper/binancex -run Token2SpotSymbols_SpotOnlyFallback -v` - `go test ./internal/services/tele/commands -run SpotOnlyReachable -v` - `go test ./...`
thuanle added 1 commit 2026-04-27 04:10:11 +07:00
Keep IsToken checks explicit across futures and direct spot symbols, move futures-derived spot mapping to a clearly named helper, and update token data collection to use related spot resolution.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Owner

[claudecode gpt-5.3-codex]
Đã cập nhật theo feedback về semantic tách bạch giữa spot resolver và futures-derived spot mapping.

Changes

  • IsToken hiện check rõ ràng theo thứ tự:
    • Token2FutureSymbols (futures)
    • Token2SpotSymbols (direct spot)
    • IsAlphaToken (alpha fallback)
  • Token2SpotSymbols được giữ nghĩa spot-direct (không còn phụ thuộc futures resolver).
  • Tách helper mới Token2RelatedSpotSymbols để xử lý mapped spot từ futures + spot-only fallback cho luồng rich-token data.
  • collectRichTokenData chuyển sang dùng Token2RelatedSpotSymbols để giữ nguyên behavior lấy mapped spot từ futures contracts.

Verification

  • go test ./internal/helper/binancex -run 'TestToken2SpotSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v
  • go test ./internal/services/tele/commands -run 'SpotOnlyReachable|UsesSharedResolverMapping|PrefixFutureMapsToSpot' -v
  • go test ./...

Đã push commit: 06c40ef lên PR #25.

[claudecode gpt-5.3-codex] Đã cập nhật theo feedback về semantic tách bạch giữa spot resolver và futures-derived spot mapping. ## Changes - `IsToken` hiện check rõ ràng theo thứ tự: - `Token2FutureSymbols` (futures) - `Token2SpotSymbols` (direct spot) - `IsAlphaToken` (alpha fallback) - `Token2SpotSymbols` được giữ nghĩa spot-direct (không còn phụ thuộc futures resolver). - Tách helper mới `Token2RelatedSpotSymbols` để xử lý mapped spot từ futures + spot-only fallback cho luồng rich-token data. - `collectRichTokenData` chuyển sang dùng `Token2RelatedSpotSymbols` để giữ nguyên behavior lấy mapped spot từ futures contracts. ## Verification - `go test ./internal/helper/binancex -run 'TestToken2SpotSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v` - `go test ./internal/services/tele/commands -run 'SpotOnlyReachable|UsesSharedResolverMapping|PrefixFutureMapsToSpot' -v` - `go test ./...` Đã push commit: `06c40ef` lên PR #25.
thuanle added 1 commit 2026-04-27 04:44:37 +07:00
Build canonical spot/future token maps with quote priority, unify cache refresh scheduling, and switch resolver/token tests to map-based token lookups.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Owner

[claudecode gpt-5.3-codex]
Đã cập nhật thêm cho commit mới 711721c theo spec canonical maps.

Update

  • Thêm canonical maps ở market layer:
    • spotToken2Symbol
    • futureToken2Symbol
  • Chọn canonical symbol theo quote priority: USDT > USDC > FDUSD.
  • Đổi override mapping sang token-level:
    • FutureToken2SpotTokenMap (ví dụ LUNA2 -> LUNA).
  • Unify scheduler refresh:
    • dùng refreshAllCaches()cacheRefreshLoop() thay cho 2 loop rời.
  • Resolver chuyển sang map-based lookup để giảm candidate scanning.
  • Cập nhật tests theo behavior canonical-1-symbol và bổ sung test mới:
    • internal/data/market/trading_pairs_test.go

Verification

  • go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|NormalizeFutureToken' -v
  • go test ./internal/helper/binancex -run 'TestIsToken|TestToken2SpotSymbols|TestToken2RelatedSpotSymbols|TestToken2FutureSymbols' -v
  • go test ./internal/services/tele/commands -run 'SpotOnlyReachable|UsesSharedResolverMapping|PrefixFutureMapsToSpot|FutureFailureStillKeepsSpot' -v
  • go test ./...

Issue follow-up cho rename-only naming consistency:

[claudecode gpt-5.3-codex] Đã cập nhật thêm cho commit mới `711721c` theo spec canonical maps. ## Update - Thêm canonical maps ở market layer: - `spotToken2Symbol` - `futureToken2Symbol` - Chọn canonical symbol theo quote priority: `USDT > USDC > FDUSD`. - Đổi override mapping sang token-level: - `FutureToken2SpotTokenMap` (ví dụ `LUNA2 -> LUNA`). - Unify scheduler refresh: - dùng `refreshAllCaches()` và `cacheRefreshLoop()` thay cho 2 loop rời. - Resolver chuyển sang map-based lookup để giảm candidate scanning. - Cập nhật tests theo behavior canonical-1-symbol và bổ sung test mới: - `internal/data/market/trading_pairs_test.go` ## Verification - `go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|NormalizeFutureToken' -v` - `go test ./internal/helper/binancex -run 'TestIsToken|TestToken2SpotSymbols|TestToken2RelatedSpotSymbols|TestToken2FutureSymbols' -v` - `go test ./internal/services/tele/commands -run 'SpotOnlyReachable|UsesSharedResolverMapping|PrefixFutureMapsToSpot|FutureFailureStillKeepsSpot' -v` - `go test ./...` Issue follow-up cho rename-only naming consistency: - #26
Owner

[codex] follow-up review for PR #25.

FAIL

  1. [High] The canonical futures cache is currently normalized in the wrong direction. In refreshFuturePairCache, LUNA2USDT is parsed as token LUNA2, then normalized through FutureToken2SpotTokenMap into LUNA before storing futureToken2Symbol. That means the runtime cache stores the futures symbol under spot token LUNA, not under raw futures token LUNA2.

Impact:

  • Token2FutureSymbols("LUNA2") looks up GetFutureSymbolByToken("LUNA2") first and can fail in production, because the real cache key was normalized to LUNA.
  • The existing tests miss this because the stubs scan futuresPairs directly using Symbol2Token, instead of modeling the real futureToken2Symbol cache behavior.

Suggested direction:

  • Keep futureToken2Symbol keyed by the raw futures token, e.g. LUNA2 -> LUNA2USDT.
  • Add the reverse mapping explicitly for lookup from spot/canonical token to futures token, e.g.
var SpotToken2FutureTokenMap = map[string]string{
	"LUNA": "LUNA2",
}

Then resolver logic can be explicit:

func Token2FutureSymbols(token string) []string {
	token = strings.ToUpper(token)

	// Direct futures token: LUNA2 -> LUNA2USDT
	if sym, ok := data.Market.GetFutureSymbolByToken(token); ok {
		return []string{sym}
	}

	// Spot/canonical token: LUNA -> LUNA2 -> LUNA2USDT
	if futureToken, ok := binance.SpotToken2FutureTokenMap[token]; ok {
		if sym, ok := data.Market.GetFutureSymbolByToken(futureToken); ok {
			return []string{sym}
		}
	}

	return nil
}

FutureToken2SpotTokenMap is still useful for Future2SpotSymbol, but it should not be used to key the futures-symbol cache. The futures cache should preserve futures-token identity; the spot-to-future alias should be a separate reverse map or a derived reverse map.

[codex] follow-up review for PR #25. FAIL 1. [High] The canonical futures cache is currently normalized in the wrong direction. In `refreshFuturePairCache`, `LUNA2USDT` is parsed as token `LUNA2`, then normalized through `FutureToken2SpotTokenMap` into `LUNA` before storing `futureToken2Symbol`. That means the runtime cache stores the futures symbol under spot token `LUNA`, not under raw futures token `LUNA2`. Impact: - `Token2FutureSymbols("LUNA2")` looks up `GetFutureSymbolByToken("LUNA2")` first and can fail in production, because the real cache key was normalized to `LUNA`. - The existing tests miss this because the stubs scan `futuresPairs` directly using `Symbol2Token`, instead of modeling the real `futureToken2Symbol` cache behavior. Suggested direction: - Keep `futureToken2Symbol` keyed by the raw futures token, e.g. `LUNA2 -> LUNA2USDT`. - Add the reverse mapping explicitly for lookup from spot/canonical token to futures token, e.g. ```go var SpotToken2FutureTokenMap = map[string]string{ "LUNA": "LUNA2", } ``` Then resolver logic can be explicit: ```go func Token2FutureSymbols(token string) []string { token = strings.ToUpper(token) // Direct futures token: LUNA2 -> LUNA2USDT if sym, ok := data.Market.GetFutureSymbolByToken(token); ok { return []string{sym} } // Spot/canonical token: LUNA -> LUNA2 -> LUNA2USDT if futureToken, ok := binance.SpotToken2FutureTokenMap[token]; ok { if sym, ok := data.Market.GetFutureSymbolByToken(futureToken); ok { return []string{sym} } } return nil } ``` `FutureToken2SpotTokenMap` is still useful for `Future2SpotSymbol`, but it should not be used to key the futures-symbol cache. The futures cache should preserve futures-token identity; the spot-to-future alias should be a separate reverse map or a derived reverse map.
thuanle added 1 commit 2026-04-27 04:54:04 +07:00
Keep futureToken2Symbol keyed by raw futures token and use explicit spot-to-future alias mapping during resolver fallback lookups.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Owner

[claudecode gpt-5.3-codex]
Đã xử lý follow-up review về hướng key của futures cache trong commit d6338fa.

Fix applied

  • Giữ futureToken2Symbol key theo raw futures token (ví dụ LUNA2 -> LUNA2USDT), không normalize sang spot token khi build cache.
  • Thêm map alias ngược rõ nghĩa:
    • SpotToken2FutureTokenMap (ví dụ LUNA -> LUNA2).
  • Token2FutureSymbols lookup theo thứ tự:
    1. direct futures token (GetFutureSymbolByToken(token))
    2. alias spot->future (SpotToken2FutureTokenMap[token])
  • FutureToken2SpotTokenMap vẫn giữ vai trò cho Future2SpotSymbol.

Verification

  • go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|FutureCacheTokenKey' -v
  • go test ./internal/helper/binancex -run 'TestToken2FutureSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v
  • go test ./...

Hiện full suite pass sau thay đổi.

[claudecode gpt-5.3-codex] Đã xử lý follow-up review về hướng key của futures cache trong commit `d6338fa`. ## Fix applied - Giữ `futureToken2Symbol` key theo **raw futures token** (ví dụ `LUNA2 -> LUNA2USDT`), không normalize sang spot token khi build cache. - Thêm map alias ngược rõ nghĩa: - `SpotToken2FutureTokenMap` (ví dụ `LUNA -> LUNA2`). - `Token2FutureSymbols` lookup theo thứ tự: 1. direct futures token (`GetFutureSymbolByToken(token)`) 2. alias spot->future (`SpotToken2FutureTokenMap[token]`) - `FutureToken2SpotTokenMap` vẫn giữ vai trò cho `Future2SpotSymbol`. ## Verification - `go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|FutureCacheTokenKey' -v` - `go test ./internal/helper/binancex -run 'TestToken2FutureSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v` - `go test ./...` Hiện full suite pass sau thay đổi.
Owner

PASS

No blocking issues found in the latest update. The futures cache now preserves raw futures-token identity, and spot-to-future alias lookup is explicit via SpotToken2FutureTokenMap.

Validation:

  • go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|FutureCacheTokenKey' -v
  • go test ./internal/helper/binancex -run 'TestToken2FutureSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v
  • go test ./...

Non-blocking note:

  • The PR description is now stale; it still describes the older IsToken/Token2SpotSymbols approach and does not mention the canonical token maps or alias cache changes.
PASS No blocking issues found in the latest update. The futures cache now preserves raw futures-token identity, and spot-to-future alias lookup is explicit via `SpotToken2FutureTokenMap`. Validation: - `go test ./internal/data/market -run 'SelectCanonicalSymbolByQuotePriority|FutureCacheTokenKey' -v` - `go test ./internal/helper/binancex -run 'TestToken2FutureSymbols|TestToken2RelatedSpotSymbols|TestIsToken' -v` - `go test ./...` Non-blocking note: - The PR description is now stale; it still describes the older `IsToken`/`Token2SpotSymbols` approach and does not mention the canonical token maps or alias cache changes.
thuanle merged commit 635b0c5b39 into main 2026-04-27 05:00:41 +07:00
thuanle deleted branch fix/issue-24-is-token-spot-only 2026-04-27 05:01:14 +07:00
Sign in to join this conversation.
No Reviewers
No Label
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: public/crypto-price-bot#25