F4 — Eigenprofil-Overlay persistieren und ausliefern (Sektion B) - #35
Merged
Conversation
Persistiert das zustandsspezifische Drift-Eigenprofil (drift_profiles, Migration 0011) und liefert es als profile_band ueber HTTP + WS aus; das Maschinen-Detail-Chart (Sektion B) rendert es als gestrichelten Erwartungskorridor neben dem statischen Normalband, mit Profil-Stand. Das ausgelieferte Band ist nachweislich die echte Detektor-Bewertungsbasis: current_median(state_key) +/- effect_size_k * noise_sigma, am Laufende des gegateten Replays weggeschrieben. Die state_key-Logik (Tagesstunde) ist als gemeinsame Funktion (baseline.state_key_for) zwischen Detektor-Lauf und Read-Expansion geteilt, NICHT dupliziert. Kein etabliertes/ausreichendes Profil -> profile_band null (graceful, FE laesst das Overlay weg); DB-CHECK noise_sigma>0 haelt diese Ehrlichkeitslinie an der Persistenzgrenze. Sektion A unberuehrt (Designstudie: Cockpit hat keinen Profil-Overlay). Backend: Migration 0011 + DriftProfile-Modell; baseline.state_key_for/ state_profiles, detector.noise_sigma; service.extract_profile + Runner-Write (upsert je Datenpunkt); reads.load_drift_profile + trend.expand_profile_band; schemas ProfileBandOut (beide Transport-Einstiege). FE: contracts/types/trend-series-Mapping, TimeSeriesChart-Korridor (data-series-2, gestrichelt), MachineTrendPanel-Profilstand. Tests: Migration 0011 up/down + CHECK/UNIQUE; Band=Detektor-Basis (unit); Read-Expansion je Zustand; graceful null; HTTP+WS befuellt; FE-Rendering/Stand. Coverage beruehrte Module 89%. GROUND_TRUTH (5/20.5/21.11) + WALKTHROUGH im selben Commit. Adversariale Multi-Agent-Review: 0 Befunde. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughDer PR führt persistierte zustandsbasierte Drift-Eigenprofile ( ÄnderungenF4 Drift Eigenprofil: Persistenz, Read-Layer und Frontend-Overlay
SequenzdiagrammsequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over DriftService,drift_profiles: Schreibpfad — nach Maschinenlauf
DriftService->>DriftReasoner: detect_drift_in_stream(samples)
DriftService->>DriftService: _persist_profiles(reasoner, topology, computed_at=end)
DriftService->>extract_profile: state_for(dp_id) → extract_profile(state)
extract_profile-->>DriftService: DataPointProfile | None
DriftService->>drift_profiles: INSERT … ON CONFLICT (data_point_id) DO UPDATE
end
rect rgba(144, 238, 144, 0.5)
Note over Client,TimeSeriesChart: Lesepfad — Trend-Anfrage
Client->>build_trend: GET /trend oder WS trend_topic
build_trend->>drift_profiles: load_drift_profile(data_point_id)
drift_profiles-->>build_trend: DriftProfile | None
build_trend->>expand_profile_band: expand_profile_band(profile, reading_buckets)
expand_profile_band-->>build_trend: ProfileBand | None
build_trend-->>Client: MachineTrendOut { profile_band }
Client->>TimeSeriesChart: series.profileBand
TimeSeriesChart->>TimeSeriesChart: SVG gestrichelter Korridor (upper/mid/lower)
end
Geschätzter Code-Review-Aufwand🎯 4 (Komplex) | ⏱️ ~60 Minuten Möglicherweise verwandte PRs
Gedicht
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/foreman/reasoners/drift/service.py`:
- Around line 406-408: The code passes `self.min_effect_size` as the
`effect_size_k` parameter to `extract_profile`, but the database has a
constraint requiring `effect_size_k > 0`. If `min_effect_size` is set to a value
less than or equal to 0, it will cause an IntegrityError during profile
insertion, rolling back the entire transaction. Add validation at the entry
point of the DriftService initialization or the replay_machine function to check
that `min_effect_size > 0` and raise a meaningful error immediately, preventing
invalid values from reaching the database operation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 139314df-813c-448a-9592-6d641a8147ed
📒 Files selected for processing (25)
GROUND_TRUTH.mddocs/WALKTHROUGH.mdfrontend/components/machine/machine-trend-panel.test.tsxfrontend/components/machine/machine-trend-panel.tsxfrontend/components/machine/time-series-chart.test.tsxfrontend/components/machine/time-series-chart.tsxfrontend/lib/api/contracts.tsfrontend/lib/machine/trend-series.test.tsfrontend/lib/machine/trend-series.tsfrontend/lib/machine/types.tsmigrations/versions/0011_drift_profiles.pysrc/foreman/db/models.pysrc/foreman/reads/queries.pysrc/foreman/reads/trend.pysrc/foreman/reasoners/drift/baseline.pysrc/foreman/reasoners/drift/detector.pysrc/foreman/reasoners/drift/service.pysrc/foreman/schemas/dashboard.pytests/integration/test_dashboard_routes.pytests/integration/test_drift_profile_persistence.pytests/integration/test_reads_trend.pytests/migrations/test_migration_0011.pytests/unit/test_dashboard_schemas.pytests/unit/test_drift_detector.pytests/unit/test_drift_service.py
Comment on lines
+406
to
+408
| profile = extract_profile( | ||
| data_point_id, topology.machine_id, state, effect_size_k=self.min_effect_size | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Default-Wert und etwaige Validierung von min_effect_size prüfen
rg -nP -C2 'DEFAULT_MIN_EFFECT_SIZE\s*[:=]' src/foreman/reasoners/drift
rg -nP -C2 'min_effect_size' src/foreman/reasoners/drift/service.pyRepository: patricznr1/foreman
Length of output: 1916
🏁 Script executed:
# Prüfe DriftReasoner-Klasse und Initialisierung
ast-grep outline src/foreman/reasoners/drift/service.py | head -50
# Suche nach extract_profile Implementierung
rg -n 'def extract_profile' src/foreman/reasoners/drift/
# Prüfe auf Validierung von min_effect_size
rg -n -B3 -A3 'min_effect_size\s*[<>=]' src/foreman/reasoners/drift/
# Suche nach DB Constraints
rg -n 'effect_size_k' src/foreman/reasoners/drift/ --type pyRepository: patricznr1/foreman
Length of output: 3637
🏁 Script executed:
# Read extract_profile function implementation
sed -n '119,154p' src/foreman/reasoners/drift/service.py
# Check runner.py context for min_effect_size parameter
sed -n '40,70p' src/foreman/reasoners/drift/runner.py
# Search for DB constraint on effect_size_k in schema or models
rg -n 'effect_size_k' src/ --type py -B2 -A2 | head -80
# Check if there's a migration or schema file defining the constraint
fd -e sql -e py | grep -i migrat | head -10Repository: patricznr1/foreman
Length of output: 8211
🏁 Script executed:
# Search in migration files for drift_profiles table definition and constraint
rg -n 'drift_profiles\|effect_size_k' migrations/ --type py -B3 -A3
# Check runner.py entry point for min_effect_size parameter source
sed -n '1,50p' src/foreman/reasoners/drift/runner.py
# Look for main entry point or CLI that calls replay_machine
rg -n 'replay_machine\|min_effect_size' src/foreman/ --type py | grep -E '(def|import|argparse|click|param)' | head -20Repository: patricznr1/foreman
Length of output: 2275
🏁 Script executed:
# Find the DB constraint on effect_size_k in migrations
cat migrations/versions/*.py | grep -A10 -B10 'effect_size_k\|drift_profiles'
# Search for CLI or main entry point that calls replay_machine
rg -n 'replay_machine' src/ --type py
# Check if there's a CLI module or main entry point
find src -name 'cli.py' -o -name '__main__.py' -o -name 'main.py' | head -5
# Look for argparse or click usage in runner or related modules
rg -n 'argparse\|`@click`\|typer' src/foreman/reasoners/drift/ --type pyRepository: patricznr1/foreman
Length of output: 3589
🏁 Script executed:
# Check main.py for CLI and min_effect_size parameter handling
cat src/foreman/main.py | grep -A50 -B5 'min_effect_size\|replay_machine\|drift'
# Check _persist_profiles and transaction context
sed -n '360,430p' src/foreman/reasoners/drift/service.py
# Search for all callers of replay_machine to see if min_effect_size is overridden
rg -n 'replay_machine' src/foreman/ --type py -A3 -B3Repository: patricznr1/foreman
Length of output: 6602
🏁 Script executed:
# Check run_machine implementation and transaction context for _persist_profiles
sed -n '235,350p' src/foreman/reasoners/drift/service.py
# Look for validation of min_effect_size at any point
rg -n 'min_effect_size\s*[<>=]|assert.*min_effect_size|if.*min_effect_size' src/foreman/reasoners/drift/ --type py
# Check if replay_machine/DriftService is called from CLI or other places
rg -n 'DriftService\|replay_machine' src/foreman/reasoners/drift/router.py --type py -B3 -A3Repository: patricznr1/foreman
Length of output: 5441
🏁 Script executed:
# Check the drift router to see how replay_machine is called and transaction context
cat src/foreman/reasoners/drift/router.py
# Check if there's a transaction context in run_machine
rg -n 'begin\|transaction\|async with' src/foreman/reasoners/drift/service.py -B2 -A2
# Look for any async session transaction context
rg -n 'AsyncSession\|sessionmaker' src/foreman/reasoners/drift/service.py -B2 -A2Repository: patricznr1/foreman
Length of output: 4074
🏁 Script executed:
# Look for background worker or scheduler that runs replay_machine
rg -n 'replay_machine\|DriftService' src/foreman/ --type py -B3 -A3 | head -80
# Check if there's a worker or task scheduler module
fd -name 'worker*' -o -name '*worker*' -o -name '*task*' src/foreman/ --type f
# Look in reasoners directory for any orchestration
ls -la src/foreman/reasoners/Repository: patricznr1/foreman
Length of output: 732
🏁 Script executed:
# Search for where replay_machine is called
rg 'replay_machine' src/foreman/ --type py -B3 -A3
# Check the reasoners __init__.py
cat src/foreman/reasoners/__init__.py
# Look for worker or scheduler
find src/foreman -name '*worker*' -o -name '*task*' -o -name '*schedule*' | grep -v __pycache__Repository: patricznr1/foreman
Length of output: 1025
🏁 Script executed:
# Look for CLI or API entry points that instantiate DriftService or call replay_machine
rg 'DriftService|replay_machine' src/foreman/ --type py | grep -v 'from foreman.reasoners.drift'
# Check if replay_machine is exposed as an API endpoint or CLI
rg '`@router`\|`@app`\|`@click`\|typer' src/foreman/reasoners/drift/ --type py
# Look for any test files that might show how min_effect_size is set
find src -name '*test*drift*' -o -name '*drift*test*' | head -5
# Check if there's validation of min_effect_size in DriftReasoner or DriftService
rg 'class DriftService|class DriftReasoner' src/foreman/reasoners/drift/ --type py -A20 | head -60Repository: patricznr1/foreman
Length of output: 4137
🏁 Script executed:
# Search for all places where replay_machine is called or DriftService is instantiated
rg 'replay_machine\(' src/foreman/ --type py -B2 -A5
# Look for any settings or configuration that passes min_effect_size
rg 'min_effect_size' src/foreman/settings.py src/foreman/config.py 2>/dev/null
# Check if there's a main entry point that calls replay_machine
find src/foreman -name 'main.py' -o -name 'cli.py' -o -name '__main__.py' | xargs grep -l 'replay_machine' 2>/dev/null
# Check the transaction context and session usage
sed -n '215,235p' src/foreman/reasoners/drift/service.pyRepository: patricznr1/foreman
Length of output: 1571
🏁 Script executed:
# Check Settings class or config for min_effect_size
rg -n 'class Settings' src/foreman/ --type py -A50 | head -100
# Look for any place that might set min_effect_size externally
rg -n 'min_effect_size\s*=' src/foreman/ --type py | grep -v 'def\|async def\|self.min_effect_size\|:.*float'
# Verify the entire flow from runner.py to service initialization
cat src/foreman/reasoners/drift/runner.pyRepository: patricznr1/foreman
Length of output: 6144
🏁 Script executed:
# Check if replay_machine is called from anywhere in the codebase
rg 'replay_machine\|DriftService(' src/foreman/ --type py -B5 -A5 | grep -v test
# Check if there are any tests that set min_effect_size
find . -path ./migrations -prune -o -name '*test*.py' -type f -print | xargs grep -l 'min_effect_size' 2>/dev/null
# Verify transaction context - check if session is in a transaction
rg 'async with.*session\|begin\|transaction' src/foreman/reasoners/drift/service.py --type py -B2 -A2Repository: patricznr1/foreman
Length of output: 338
🏁 Script executed:
# Check the test files for min_effect_size usage
cat ./tests/unit/test_drift_service.py | head -200
# Also check the relevance filter test
cat ./tests/unit/test_relevance_filter.py | grep -A10 -B10 'min_effect_size'
# Check if there's any validation or guards in the code
rg 'min_effect_size.*<|min_effect_size.*<=' src/foreman/reasoners/drift/ --type pyRepository: patricznr1/foreman
Length of output: 9185
🏁 Script executed:
# Final verification: find ALL instantiations of DriftService or calls to replay_machine
rg -n 'DriftService\(' src/foreman/ tests/ --type py
# Check if there are any dataclass/model validators
rg -n '`@validator`\|`@field_validator`' src/foreman/reasoners/drift/ --type pyRepository: patricznr1/foreman
Length of output: 225
effect_size_k-Validierung: Defensives Programmieren gegen potenzielle Constraint-Verletzung.
effect_size_k wird aus self.min_effect_size befüllt; der DB-CHECK effect_size_k > 0 weist <= 0 ab. Momentan ist DEFAULT_MIN_EFFECT_SIZE = 3.0 (Final), und replay_machine standardisiert ebenfalls auf diesen Wert. Jedoch: Falls min_effect_size programmatisch mit <= 0 gesetzt wird (möglich via replay_machine-Parameter, kein Config-Override-Mechanismus gefunden), würde _upsert_profile beim DB-INSERT einen IntegrityError werfen. Da _persist_profiles im selben Try-Block wie die Drift-Event-Persistierung läuft, rollte der komplette run_machine-Lauf inkl. bereits emittierter Events zurück.
Empfehlung: Validierung min_effect_size > 0 am Eingang von replay_machine oder DriftService.__init__ hinzufügen, um das Constraint defensiv zu schützen und eine aussagekräftige Fehlermeldung früh zu geben.
🤖 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 `@src/foreman/reasoners/drift/service.py` around lines 406 - 408, The code
passes `self.min_effect_size` as the `effect_size_k` parameter to
`extract_profile`, but the database has a constraint requiring `effect_size_k >
0`. If `min_effect_size` is set to a value less than or equal to 0, it will
cause an IntegrityError during profile insertion, rolling back the entire
transaction. Add validation at the entry point of the DriftService
initialization or the replay_machine function to check that `min_effect_size >
0` and raise a meaningful error immediately, preventing invalid values from
reaching the database operation.
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
1 participant
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.
Was
Persistiert das zustandsspezifische Drift-Eigenprofil (Reasoner #2) und liefert es als
profile_bandüber HTTP + WS aus; das Maschinen-Detail-Chart (Sektion B) rendert es als gestrichelten Erwartungskorridor neben dem statischen Normalband, mit Profil-Stand. Schließt den F4-Anschlusspunkt (profile_bandwar reserviert/null).Die Ehrlichkeitslinie (Kern dieser Aufgabe)
Das ausgelieferte Band ist nachweislich die echte Detektor-Bewertungsbasis, keine Read-Rekonstruktion:
median(state_key) +/- effect_size_k * noise_sigma—current_medianje Zustand + die eingefrorene robustenoise_sigma(MAD×1.4826) +effect_size_k = min_effect_size(3.0), am Laufende des gegateten Replays weggeschrieben.state_key-Logik (Tagesstunde) ist geteilte Funktion (reasoners/drift/baseline.state_key_for) zwischen Detektor-Lauf und Read-Expansion — nicht dupliziert (sonst falscher Zustands-Korridor).profile_bandnull (graceful, FE lässt das Overlay weg). DB-CHECKnoise_sigma > 0hält diese Linie an der Persistenzgrenze; Zustände mit < 10 Samples fehlen ehrlich.Umfang (25 Dateien)
Backend — Migration
0011+DriftProfile-Modell (UNIQUE/CHECK/FK CASCADE);baseline.state_key_for/state_profiles,detector.noise_sigma;service.extract_profile+ Runner-Upsert;reads.load_drift_profile+trend.expand_profile_band;schemas.ProfileBandOut(beide Transport-Einstiege).Frontend —
contracts/types/trend-series-Mapping (snake→camel, Fenster-Merge),TimeSeriesChart-Korridor (data-series-2, gestrichelt, yScale-Einbezug),MachineTrendPanel-Profilstand.Doku — GROUND_TRUTH §5/§20.5/§21.11 + WALKTHROUGH im selben Commit.
Gates
next build✓.Pflicht-Tests (alle grün)
Runner persistiert Profil · Band = Detektor-Basis · Read-Expansion wählt korrekten Zustand · graceful null · HTTP+WS befüllt · Migration up/down · FE rendert unterscheidbar + Profil-Stand.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests