Richard/PolyTraderSharp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
Files
ad6c676e605636ee666b7f061bae2000597f1e2d
PolyTraderSharp/UMSETZUNGSPLAN-Fable-Review-Fixes.md
T
RichardandClaude Opus 4.8 c091e79869 Docs: Testabdeckung + K3-Korrektur im Plan vermerkt 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-09 19:42:00 +02:00

11 KiB
Raw Blame History

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).

Fortschritt

  • ✅ Slice 0 – IClobClient-Seam + FakeClobClient (verhaltensneutral).
  • ✅ Slice 1 – K1/H2/H1: atomarer Claim, Cleanup+Engine schonen Leitern, Floor-Robustheit. 8 Tests.
  • ✅ Slice 2 – K2: Startup-Reconciliation (GetOpenOrders ohne assetId = alle). 3 Tests.
  • ✅ Slice 3 – K3 (System-SELL vom Ownership-Check ausgenommen + Resolved-Cache) + M5 (Demo-Score-Anzeige, stündl. Auto-Pause). 5 Tests.
  • ✅ Slice 4 – H4 (RoundToTick + Dust-Abbruch), M1 (GlobalPnl im Guard), M2 (TokenId), M3-min (serverseitiges Max + lauter Fehlschlag), M4 (Parser 9999), M6 (Fees in Orders), Doku. 10 Tests.
  • ✅ Slice 5 – H3: BUY-Skip während ExitPending (Entscheidung A).
  • ✅ Slice 6 – SnapshotService entfernt, Demo-Balance/PnL-Reconciliation, Settings-Validierung (IsLadderConfigInverted + Load-Warnung). 3 Tests.

Stand: 244 Tests grün, Build/Smoke grün.

Nachgelagerte Testabdeckung (nach dem K3-Fund)

  • Engine-Integrationstests (CopyTradingEngineTests, gemockter CLOB): H3 BUY-Skip, Doppel-SELL-Guard, K3 System-Close, Fremd-Trader-Reject, H2 Cleanup-schont-Leiter (+Kontrast). Engine _clob→IClobClient, ProcessAccountOrderAsync internal.
  • ⚠️ K3-Korrektur: Der Slice-3-Fix sass am falschen Ort (downstream ~Z.643). Der echte Ownership-Check ist der frühe inPortfolio-Lookup (~Z.437, p.SourceTraderId == signal.TraderId), der System-Signale schon vorher mit early return abwies. Jetzt am richtigen Ort via IsAuthorizedSell – vom Engine-Test aufgedeckt.
  • K1a-Test (TraderMonitorServiceTests): Cleanup cancelt Leiter-Order nicht (aktive Leiter) bzw. cancelt sie ohne Leiter. _clob→IClobClient, CleanupStaleOpenOrdersAsync internal.

Alle 3 kritischen + 4 hohen Bugs sind jetzt durch Tests abgesichert (K1a/K1b/K2/K3/H1/H2/H3/H4).

Bewusst aufgeschobene Follow-ups (Live-Verifikation/Risiko)

  • M3 Autoincrement-Migration: TradeId auf DB-Autoincrement umstellen – Schema-Änderung an der Trade-Persistenz, erst im Zielland live verifizieren. (M3-Minimum ist umgesetzt.)
  • PersistenceService-Dedup-Zeitfenster: Exists(AccountId,TokenId) blockt legit Re-Entries; robuster Fix (z.B. OpenedAt-basiert) braucht Live-Daten – Duplikat-Schutz nicht unverifiziert brechen.
  • Perf: UpsertLive-Dirty-Check (Schreib-Amplifikation) und Leiter-Parallelität – laut Fable bei aktueller Größe unkritisch.
  • M6/K2: fee-signierte Orders bzw. /data/orders ohne asset_id sind API-gated → im Zielland verifizieren.

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 View Git Blame Copy Permalink