Docs: Umsetzungsplan Fable-Review-Fixes (Slices 0-6)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
429a05f885
commit
bb929fa23a
@@ -0,0 +1,139 @@
|
|||||||
|
# Umsetzungsplan: Fable-Code-Review-Fixes (Copytrading)
|
||||||
|
|
||||||
|
> Basis: Fable-5-Review nach Umsetzung des Rentabilitätsplans (Stand 2026-07-08).
|
||||||
|
> Ausgangslage: 207 Tests grün, Build/Smoke grün. Die Logic/-Klassen sind laut Review
|
||||||
|
> sauber; die Lücken liegen im **Zusammenspiel** von SELL-Leiter, Engine und den
|
||||||
|
> Hintergrund-Services (TraderMonitorService).
|
||||||
|
|
||||||
|
## Arbeitsgrundsätze (für jeden Slice)
|
||||||
|
|
||||||
|
1. **`.agents/rules/clob.md`:** vor jedem CLOB-nahen Slice ein Commit als Rollback-Punkt;
|
||||||
|
Preis-/Zustandslogik pur in `SellLogic`/`CopyTradingRisk` + neue Tests; Service-Interaktionen
|
||||||
|
(Cleanup überspringt Leiter etc.) mit kleinem **Integrationstest über gemockten CLOB-Client**.
|
||||||
|
2. Nach jedem Slice: `dotnet build` + `dotnet test` (alle grün) + `--smoke-ui` grün, dann commit+push.
|
||||||
|
3. Ein Slice = eine kohärente Einheit = ein Commit. Reihenfolge unten folgt Fables Empfehlung.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 0 (Prereq): Testbarkeit — `IClobClient`-Interface
|
||||||
|
**Warum zuerst:** K1/H2/K2 brauchen Integrationstests mit gemocktem CLOB. `PolymarketClobClient`
|
||||||
|
ist heute eine konkrete Klasse ohne Interface → nicht mockbar.
|
||||||
|
|
||||||
|
- Interface `IClobClient` (Core) mit den von Leiter/Reconciliation genutzten Methoden:
|
||||||
|
`PlaceOrderAsync`, `CancelOrderAsync`, `GetOpenOrdersAsync`, `CancelConflictingOrdersAsync`.
|
||||||
|
- `PolymarketClobClient : IClobClient`. DI zusätzlich `IClobClient → PolymarketClobClient`.
|
||||||
|
- `SellLadderService`/Reconciliation gegen `IClobClient` typisieren (Engine kann vorerst konkret bleiben).
|
||||||
|
- **Verhaltensneutral, keine Logikänderung.** Ermöglicht `FakeClobClient` im Testprojekt.
|
||||||
|
- Tests: keine neuen fachlichen; Build grün genügt.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 1: „Wer darf Leiter-Orders anfassen" (H1 + K1 + H2) 🔴🟠
|
||||||
|
Kernthema: Leiter-Order darf nur von der Leiter angefasst/gecancelt werden.
|
||||||
|
|
||||||
|
- **H1 — Atomarer Claim:** In `SellLadderService.StartLadderAsync` als ERSTES
|
||||||
|
`if (!_copyState.ExitLadders.TryAdd(key, placeholder)) return false;` → macht ALLE Aufrufer
|
||||||
|
(Engine-SELL + ProfitTarget) idempotent. Bei Fehlschlag der Order den Key wieder entfernen.
|
||||||
|
- **K1 — Cleanup überspringt Leitern:** In `TraderMonitorService.CleanupStaleOpenOrdersAsync`
|
||||||
|
Keys mit `_copyState.ExitLadders.ContainsKey(key)` überspringen (`continue`).
|
||||||
|
- **K1 — Floor-Robustheit:** In `SellLadderService.ProcessLadderAsync` am Floor NICHT dauerhaft
|
||||||
|
früh zurückkehren, sondern periodisch via `GetOpenOrdersAsync` prüfen, ob die Floor-Order noch
|
||||||
|
ruht; wenn nicht → am Floor neu platzieren (+ `PendingOrderTimestamps` refreshen).
|
||||||
|
- **H2 — Engine-Cancel schont Leiter:** Den Pre-Signal-`CancelConflictingOrdersAsync`-Aufruf der
|
||||||
|
Engine überspringen, wenn `_copyState.ExitLadders.ContainsKey(key)` (oder hinter den
|
||||||
|
ExitPending-Check verschieben).
|
||||||
|
- Tests: Integrationstest (FakeClob) — Cleanup cancelt KEINE Leiter-Order; zwei parallele
|
||||||
|
StartLadder-Aufrufe → nur eine Leiter; Floor-Order weg → Leiter platziert neu. Pure: ggf.
|
||||||
|
Floor-Recheck-Entscheidung.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 2: Neustart-Reconciliation (K2) 🔴
|
||||||
|
Ruhende GTC-Leiter-/Maker-Orders überleben Neustarts, der Verwaltungszustand nicht.
|
||||||
|
|
||||||
|
- Beim Modul-Start je **Live-Account** alle offenen CLOB-Orders via `GetOpenOrdersAsync` abrufen und
|
||||||
|
pauschal canceln (deterministisch; die Engine entscheidet danach sauber neu). Kein Leiter-Rebuild.
|
||||||
|
- Ort: eigener Startup-Schritt im Modul (z. B. in `TraderMonitorService`-Warmup oder als kurzer
|
||||||
|
`IHostedService`), NACH der State-Hydration, VOR dem ersten Signal-Processing.
|
||||||
|
- Umfangreiches Logging (welche Orders gecancelt).
|
||||||
|
- Tests: Integrationstest (FakeClob) — für jeden offenen Order-Eintrag wird Cancel gerufen.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 3: Demo-Resolution + Demo-Score (K3 + M5) 🔴🟡
|
||||||
|
Sonst ist die Demo-Validierungsphase (auf der die Zielland-Strategie beruht) wertlos.
|
||||||
|
|
||||||
|
- **K3 — System-Signale (TraderId==0) vom Ownership-Check ausnehmen:** In der Engine SELL-Pre-Flight
|
||||||
|
(`p.SourceTraderId == signal.TraderId`) den Fall `signal.TraderId == 0` zulassen (System-Close bei
|
||||||
|
Marktauflösung). Zusätzlich „bereits als resolved erkannt"-Cache, damit ein Markt nur einmal
|
||||||
|
verarbeitet wird (verhindert 30-s-Loop-Spam + API-Last).
|
||||||
|
- **M5 — Demo-Score & schnellerer Auto-Pause:** Copy-Score getrennt für Demo (Anzeige/Validierung)
|
||||||
|
und Live (Pausieren) berechnen; der Kill-Switch filtert weiterhin `!IsDemo`, aber die Demo-Kennzahlen
|
||||||
|
füllen die Spalten. Zusätzlich stündlicher Light-Check nur für die Pause-Regel (statt nur alle 12 h).
|
||||||
|
- Tests: Ownership-Ausnahme (Engine), Resolved-Cache (pure). Demo/Live-Score-Trennung ist Job-Logik.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 4: Kleine, klar umrissene Fixes (H4 + M1 + M2 + M3 + M4 + M6 + Doku) 🟠🟡🟢
|
||||||
|
Jeweils klein und abgegrenzt — in einem oder zwei Commits.
|
||||||
|
|
||||||
|
- **H4 — Dust-Reject-Schleife:** (a) Leiter-Preis vor der USDC-Berechnung auf Tick runden
|
||||||
|
(`Math.Round(next, 3)`, zentral in `SellLogic`); (b) Abbruch in `ProcessLadderAsync`:
|
||||||
|
`pos.Size < CopyTradingRisk.MinShares` → Leiter beenden, `ExitPending=false`, Dust loggen.
|
||||||
|
Pure Tests für Rundung + Abbruch.
|
||||||
|
- **M1 — GlobalPnl-Doppelzählung:** In `TraderMonitorService` (~Z.910 und ~Z.961) das
|
||||||
|
`GlobalPnl += realizedPnl` INNERHALB des `_processedClosures`-Guards buchen (wie in
|
||||||
|
`PollClosedAccountsAsync` bereits korrekt).
|
||||||
|
- **M2 — TokenId in Live-Close-Records:** In beiden Live-Close-Records (~Z.922-940 und ~Z.972-990)
|
||||||
|
`TokenId = removedPos.TokenId` setzen (der 0.4-Fix erwischte nur den Demo-Pfad).
|
||||||
|
- **M3 — TradeId robust:** `ClosedTrade.TradeId` auf DB-Autoincrement (`ValueGeneratedOnAdd`)
|
||||||
|
umstellen + Code-Vergabe (`GetNextTradeId`) entfernen + Migration. Eliminiert die stille
|
||||||
|
PK-Kollisions-Fehlerklasse und den teuren Full-Table-`Max()`-Startup in `Program.cs`.
|
||||||
|
(Alternative/Minimum: serverseitiges `Max()` + lauter Fehlschlag statt `catch {}`.)
|
||||||
|
- **M4 — Parser-Default:** `MongoExportParser` ProfitTarget-Fallback `50m → 9999m` (sonst schaltet
|
||||||
|
ein erneuter `--migrate-json`-Lauf Take-Profit unbeabsichtigt scharf). Pure Test.
|
||||||
|
- **M6 — Fee in signierte Orders:** An den Callsites (Engine-BUY, Leiter, PreRedeem)
|
||||||
|
`actualFeeBps` aus `FeeModel`/`MarketData.TakerFeeBps` an `PlaceOrderAsync` durchreichen.
|
||||||
|
Verifikation im Zielland, aber die Verdrahtung jetzt.
|
||||||
|
- **Doku — Stale [Description]:** `SellFloorPct` ist verdrahtet (nicht „Phase 0.1 offen");
|
||||||
|
`ProfitTarget`-Text nicht mehr „folgt in Phase 0.3". Texte aktualisieren (Richard verlässt sich drauf).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 5: H3 — BUY während ExitPending 🟠
|
||||||
|
Re-buyt der Master, während unsere Leiter verkauft, kauft die Engine normal zu → die Leiter verkauft
|
||||||
|
danach `pos.Size` inkl. neuer Shares zum alten Floor.
|
||||||
|
|
||||||
|
**ENTSCHEIDUNG (Richard, 2026-07-08): Variante A — BUYs skippen, solange `ExitPending`.**
|
||||||
|
Während des Ausstiegs keine Zukäufe; die Leiter verkauft die Position sauber zu Ende.
|
||||||
|
|
||||||
|
- Umsetzung: In der Engine BUY-Pre-Flight früh prüfen —
|
||||||
|
`if (account.OpenPositions.TryGetValue(signal.TokenId, out var p) && p.ExitPending) { log + return; }`.
|
||||||
|
(Spiegelt die bestehende Double-Sell-Guard-Logik, nur für den BUY-Pfad.)
|
||||||
|
- Umfangreiches Logging (verworfener BUY während aktivem Exit inkl. TokenId/TraderId).
|
||||||
|
- Tests: Engine-BUY-Pfad überspringt, solange `ExitPending`; nach Leiter-Ende (ExitPending=false)
|
||||||
|
wird ein neuer BUY wieder normal ausgeführt.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Slice 6: Rest nach Gelegenheit (🟢 Perf/Doku)
|
||||||
|
- **PersistenceService-Dedup:** `Exists(AccountId, TokenId)` blockt legitime Re-Entries (HF-Alltag) →
|
||||||
|
Dedup-Schlüssel um Zeitfenster ergänzen. (Copy-Score untererfasst sonst.)
|
||||||
|
- **Demo-Balance vs. PnL:** Balance sollte `exitUsd − ExitFee` gutschreiben (und der BUY die Entry-Fee
|
||||||
|
abziehen), damit Σ(Balance-Änderungen) = Σ(PnL). Aktuell driftet es um die Fees.
|
||||||
|
- **Settings-Validierung:** `MaxPriceDifference% > SellFloorPct` → Leiter startet unter dem Floor
|
||||||
|
(sofortige „Floor erreicht"-Notification). UI-Warnung/Validierung.
|
||||||
|
- **Perf — Schreib-Amplifikation:** `PollLiveAccountsAsync` `UpsertLive` je Position alle 30 s →
|
||||||
|
Dirty-Check (nur bei Änderung) oder Batch.
|
||||||
|
- **Perf — Leitern seriell:** `ProcessLadderAsync` pro Tick seriell → begrenzte Parallelität + Timeout.
|
||||||
|
- **Totcode:** `services/SnapshotService.cs` entfernen (nirgends registriert) oder bewusst reaktivieren.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Empfohlene Reihenfolge (Fable)
|
||||||
|
`Slice 0` (Test-Infra) → `Slice 1` (K1+H2+H1) → `Slice 2` (K2) → `Slice 3` (K3+M5) →
|
||||||
|
`Slice 4` (H4/M1/M2/M3/M4/M6/Doku) → `Slice 5` (H3, nach Entscheidung) → `Slice 6` (Rest).
|
||||||
|
|
||||||
|
**Als korrekt bestätigt (nicht anfassen):** Logic/-Klassen sauber/verhaltenstreu; ExitPending
|
||||||
|
EF-ignoriert; TotalFees gemappt; closed_trades-Indizes vorhanden; SellLadderService als
|
||||||
|
Singleton+Hosted (eine Instanz); Copy-Score-Find serverseitig; SELL-Spam-Blockade seitensensitiv.
|
||||||
Reference in New Issue
Block a user