Skip to content

fix(repository): set where_added for ttfb filter#58

Open
fergusfinn wants to merge 1 commit intomainfrom
fix/TTN-3113-where-added-flag
Open

fix(repository): set where_added for ttfb filter#58
fergusfinn wants to merge 1 commit intomainfrom
fix/TTN-3113-where-added-flag

Conversation

@fergusfinn
Copy link
Contributor

Summary

  • set where_added = true in the max_duration_to_first_byte_ms filter branch when it introduces the first WHERE clause
  • preserve the existing query builder pattern so later filters append with AND

Testing

  • DATABASE_URL=postgresql:///postgres cargo test

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 10, 2026 11:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in RequestFilter::build_query where the max_duration_to_first_byte_ms filter branch pushed a WHERE clause to the SQL query but failed to set where_added = true, causing any filter applied after it to also emit WHERE instead of AND, producing invalid SQL.

Changes:

  • Adds where_added = true in the else branch of the max_duration_to_first_byte_ms filter block, making it consistent with every other filter branch in build_query.
Comments suppressed due to low confidence (1)

src/repository.rs:218

  • The test suite covers every other filter field (including min_duration_ms/max_duration_ms) but there are no unit tests for min_duration_to_first_byte_ms or max_duration_to_first_byte_ms. Without a dedicated test:
  1. The exact bug this PR fixes (missing where_added = true) would not have been caught by the existing tests.
  2. A future regression in the TTFB filter branch would similarly go undetected.

Please add tests analogous to test_duration_filters that exercise:

  • max_duration_to_first_byte_ms as the sole filter (verifying it emits WHERE rather than AND).
  • Both min_duration_to_first_byte_ms and max_duration_to_first_byte_ms together (verifying the first gets WHERE and the second gets AND).
  • max_duration_to_first_byte_ms combined with another filter (e.g., correlation_id) to verify the AND path still works correctly.
        if let Some(max_duration_to_first_byte) = self.max_duration_to_first_byte_ms {
            if where_added {
                query.push(" AND ");
            } else {
                query.push(" WHERE ");
                where_added = true;
            }
            query.push("res.duration_to_first_byte_ms <= ");
            query.push_bind(max_duration_to_first_byte);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants