fix(db): hook_perf_baseline filters wrong column (#1494 residual) - #1500
Conversation
Companion fix to #1494 (which fixed the schema accept). dog-fooder-11eb's verdict (state/evidence/followups-1490-1491-1493/REPORT.md) flagged a SECOND bug after the schema fix landed: the view created by migration 056 filtered `WHERE kind = 'hook.delivery'`, but the actual emitted genie_runtime_events row carries: subject = 'hook.delivery' ← what we want to filter on kind = 'system' ← what migration 056 incorrectly filters on data->>'_kind' = 'span' ← span marker Net effect: rows accepted into the table never appeared in the view, so `genie doctor --perf` and `hook_perf_baseline` queries always returned empty. The hookify-perf-foundation telemetry (#1485) was inert. Fix: migration 057 replaces the view to filter on `subject='hook.delivery' AND data->>'_kind' = 'span'`. Empirical: BEFORE migration: SELECT count(*) FROM hook_perf_baseline → 0 AFTER migration: SELECT count(*) FROM hook_perf_baseline → 5 Daemon-mode hook telemetry now actually populates the view that `genie doctor --perf` reads from. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a bug in the hook_perf_baseline view by correcting the filter criteria to use the subject column instead of kind and ensuring only completed spans are included. Feedback was provided to improve the resilience of the duration_ms calculation by using a safe numeric cast to prevent the view from failing on malformed data.
| COALESCE(data->>'event', '<unknown>') AS event_name, | ||
| COALESCE(data->>'tool', '<none>') AS tool_name, | ||
| COALESCE(data->>'hook_name', '<unknown>') AS handler_name, | ||
| NULLIF(data->>'duration_ms', '')::numeric AS duration_ms, |
There was a problem hiding this comment.
The direct cast ::numeric is not safe and will cause the entire view query to fail if data->>'duration_ms' contains a non-numeric value (e.g., from malformed event data). This would make the hook_perf_baseline view and any dependent features unusable until the bad data is cleared.
To make the view more resilient, it's better to validate the value before casting. A CASE statement with a regular expression can be used to safely attempt the conversion, coercing invalid values to NULL. These NULL values will then be filtered out by the outer WHERE duration_ms IS NOT NULL clause.
(CASE WHEN data->>'duration_ms' ~ '^[0-9]+$' THEN (data->>'duration_ms')::numeric END) AS duration_ms,#1494 residual) Tests were inserting rows with kind='hook.delivery' (the OLD filter the view no longer matches) and asserting they appeared in hook_perf_baseline. After migration 057 changes the view to filter on subject='hook.delivery' AND data._kind='span', tests must insert rows in that shape. Updates: - DELETE sweep now matches both subject AND legacy kind for cleanup - INSERT statements add subject='hook.delivery', kind='system' (matches real emitter shape per 11eb verdict) - Test fixture data JSON now includes _kind: 'span' so the new view's WHERE data->>'_kind' = 'span' clause matches Validation: 5/5 src/db/migrations/hook-perf-baseline.test.ts pass.
1 participant
Companion fix to #1494. dog-fooder-11eb's verdict flagged that migration 056 filtered
WHERE kind = 'hook.delivery'but actual rows carrysubject='hook.delivery',kind='system',data._kind='span'. View was returning 0 rows regardless of dispatch activity.Migration 057 replaces the view to filter on
subject='hook.delivery' AND data->>'_kind'='span'.Empirical
SELECT count(*) FROM hook_perf_baseline→ 0SELECT count(*) FROM hook_perf_baseline→ 5genie doctor --perfand the hookify-perf-foundation telemetry now actually work.