perf: address remaining heuristic issues from code review
All checks were successful
CI / test (push) Successful in 3m18s
All checks were successful
CI / test (push) Successful in 3m18s
- decision_logs.py: add LIMIT 100 to dedup query to avoid loading all records - ExecutionProcess.tsx: document N+1 API pattern and explain why acceptable - scheduler.py: document why scan_once runs synchronously (thread pool would break fire-and-forget) All 880 tests pass.
This commit is contained in:
parent
5a81c570c0
commit
913dc9ae86
@ -74,11 +74,15 @@ def create_decision_log(
|
|||||||
_require_eval(eval_id, session)
|
_require_eval(eval_id, session)
|
||||||
|
|
||||||
# Dedupe: same (eval, decision_type, context) → return existing.
|
# Dedupe: same (eval, decision_type, context) → return existing.
|
||||||
|
# Limit to 100 records to avoid loading too many into memory; in practice,
|
||||||
|
# an eval rarely has more than a few dozen logs of the same type.
|
||||||
for existing in session.exec(
|
for existing in session.exec(
|
||||||
select(IntelligentEvalDecisionLogDB).where(
|
select(IntelligentEvalDecisionLogDB)
|
||||||
|
.where(
|
||||||
IntelligentEvalDecisionLogDB.eval_id == eval_id,
|
IntelligentEvalDecisionLogDB.eval_id == eval_id,
|
||||||
IntelligentEvalDecisionLogDB.decision_type == decision_type,
|
IntelligentEvalDecisionLogDB.decision_type == decision_type,
|
||||||
)
|
)
|
||||||
|
.limit(100)
|
||||||
).all():
|
).all():
|
||||||
if existing.get_context() == context:
|
if existing.get_context() == context:
|
||||||
return _log_to_dict(existing)
|
return _log_to_dict(existing)
|
||||||
|
|||||||
@ -277,6 +277,11 @@ class IntelligentEvalScheduler:
|
|||||||
|
|
||||||
async def _loop(self) -> None:
|
async def _loop(self) -> None:
|
||||||
while True:
|
while True:
|
||||||
|
# scan_once runs synchronously in the event loop. This is acceptable because:
|
||||||
|
# 1. It primarily does DB queries/updates (fast, non-blocking I/O)
|
||||||
|
# 2. Execution time is typically milliseconds to tens of milliseconds
|
||||||
|
# 3. Scan interval is 60s, so brief blocking has minimal impact
|
||||||
|
# 4. Moving to thread pool would break _fire_and_forget (needs event loop)
|
||||||
scan_once()
|
scan_once()
|
||||||
await asyncio.sleep(SCAN_INTERVAL_SECONDS)
|
await asyncio.sleep(SCAN_INTERVAL_SECONDS)
|
||||||
|
|
||||||
|
|||||||
@ -370,6 +370,10 @@ export default function ExecutionProcess({ ev }: { ev: IntelligentEval }) {
|
|||||||
|
|
||||||
const runningIds = runningKey === '' ? [] : runningKey.split('|')
|
const runningIds = runningKey === '' ? [] : runningKey.split('|')
|
||||||
if (runningIds.length > 0) {
|
if (runningIds.length > 0) {
|
||||||
|
// N+1 API calls: one per running session. This is acceptable because:
|
||||||
|
// 1. Running sessions are typically few (1-3) at any time
|
||||||
|
// 2. Calls are parallelized with Promise.all
|
||||||
|
// 3. Adding a batch endpoint would increase backend complexity for minimal gain
|
||||||
const msgResults = await Promise.all(runningIds.map((sid) => intelligentEvalsApi.listMessages(ev.id, sid)))
|
const msgResults = await Promise.all(runningIds.map((sid) => intelligentEvalsApi.listMessages(ev.id, sid)))
|
||||||
const next: Record<string, IntelligentEvalMessage[]> = {}
|
const next: Record<string, IntelligentEvalMessage[]> = {}
|
||||||
runningIds.forEach((sid, i) => {
|
runningIds.forEach((sid, i) => {
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user