fix(db_statement_summary): bound INSERT summary length like the SELECT path

[chuenchen309]

Summary

summarize_query keeps DB-statement span messages within MAX_QUERY_MESSAGE_LENGTH (80). The SELECT path enforces this by ending in truncate(summary, MAX_QUERY_MESSAGE_LENGTH), and the fallback path is bounded too — but the INSERT branch returned its f-string directly:

return f'INSERT INTO {table} {truncate(columns, 25)} VALUES {truncate(values, 25)}'

columns/values are truncated, but table is not, so a long (e.g. schema-qualified) table name pushes the summary past the limit.

Reproduction

>>> from logfire._internal.db_statement_summary import summarize_query
>>> q = 'INSERT INTO "analytics"."very_long_events_table_name_for_testing" (a, b, c, d) VALUES (1, 2, 3, 4)'
>>> len(summarize_query(q))
112   # > MAX_QUERY_MESSAGE_LENGTH (80)

The equivalent SELECT with the same long table stays bounded (~74 chars).

Fix

Wrap the INSERT result in the same final truncate(..., MAX_QUERY_MESSAGE_LENGTH) that the SELECT path already applies, so every branch stays bounded.

Tests

Added test_insert_long_table; the existing test_insert (short table) is unchanged. Full tests/test_db_statement_summary.py passes (24), and ruff format/check + pyright are clean on the touched files.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bc4129d4-5184-4dfe-83c5-eac204644bd1

📥 Commits

Reviewing files that changed from the base of the PR and between 96d96bb and 2156578.

📒 Files selected for processing (1)

• tests/test_db_statement_summary.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• pydantic/logfire (manual)
• pydantic/pydantic-ai (manual)
• pydantic/platform (auto-detected)
• pydantic/pydantic (auto-detected)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/test_db_statement_summary.py

---

📝 Walkthrough

Walkthrough

INSERT query summaries now apply the shared truncate function to the complete generated message, enforcing MAX_QUERY_MESSAGE_LENGTH after composing the table, columns, and values. A regression test covers an INSERT statement with a long schema-qualified table name and verifies that the returned summary remains within the configured limit.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: bounding INSERT summaries like the SELECT path.
Description check	✅ Passed	The description matches the change set and explains the INSERT truncation fix and regression test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@logfire/_internal/db_statement_summary.py`:
- Around line 82-85: The INSERT summary truncation in truncate must reserve one
character for the ellipsis so the final result never exceeds
MAX_QUERY_MESSAGE_LENGTH. Update logfire/_internal/db_statement_summary.py lines
82-85 accordingly, then revise tests/test_db_statement_summary.py lines 158-168
to assert len(summary) <= MAX_QUERY_MESSAGE_LENGTH and update the expected
literal for the corrected output.

🪄 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: 8232872b-3d75-4136-88b1-5479f41f1aee

📥 Commits

Reviewing files that changed from the base of the PR and between db9859a and 60dd04a.

📒 Files selected for processing (2)

• logfire/_internal/db_statement_summary.py
• tests/test_db_statement_summary.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• pydantic/logfire (manual)
• pydantic/pydantic-ai (manual)
• pydantic/platform (auto-detected)
• pydantic/pydantic (auto-detected)

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

The INSERT length-bound contract still has an off-by-one gap.

truncate(..., 80) currently produces 81 characters when truncating, while the regression test does not assert the configured bound directly.

• logfire/_internal/db_statement_summary.py#L82-L85: fix truncate to reserve one character for …, ensuring the final result is no longer than 80.
• tests/test_db_statement_summary.py#L158-L168: assert len(summary) <= MAX_QUERY_MESSAGE_LENGTH and update the expected literal after correcting the helper.

🧰 Tools

🪛 Ruff (0.15.21)

[error] 83-83: Possible SQL injection vector through string-based query construction

(S608)

📍 Affects 2 files

• logfire/_internal/db_statement_summary.py#L82-L85 (this comment)
• tests/test_db_statement_summary.py#L158-L168

🤖 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 `@logfire/_internal/db_statement_summary.py` around lines 82 - 85, The INSERT
summary truncation in truncate must reserve one character for the ellipsis so
the final result never exceeds MAX_QUERY_MESSAGE_LENGTH. Update
logfire/_internal/db_statement_summary.py lines 82-85 accordingly, then revise
tests/test_db_statement_summary.py lines 158-168 to assert len(summary) <=
MAX_QUERY_MESSAGE_LENGTH and update the expected literal for the corrected
output.

[chuenchen309]

Added an explicit len(result) <= MAX_QUERY_MESSAGE_LENGTH + 1 assertion to the test to make the bound self-documenting.

On the + 1: the 81-vs-80 is a pre-existing property of truncate() itself — it returns 2 * (length // 2) + 1 characters for an even length, so truncate(s, 80) yields up to 81. That behavior is shared by the SELECT path, which already ends in truncate(summary, MAX_QUERY_MESSAGE_LENGTH). The goal of this PR is only to make the INSERT branch honor the same bound the SELECT branch already does; changing truncate() to reserve a slot for the ellipsis would alter the output of every existing SELECT summary and break the current exact-match tests (test_long_select, test_one_join_long, etc.), so it is out of scope here. Happy to tighten truncate() in a separate PR if that is wanted.

(The Ruff S608 "SQL injection" hint is a false positive — this f-string builds a truncated display summary for a span message, it is never executed as SQL.)

[coderabbitai]

CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set chat.allow_non_org_members: true in your configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[chuenchen309]

Good catch that truncate(summary, MAX_QUERY_MESSAGE_LENGTH) can return MAX + 1 chars — but that's a property of the shared truncate() helper, not something this PR introduces: for an even length, truncate returns 2 * (length // 2) + 1 (so 80 → 81). The main/SELECT path already bounds its summary the exact same way (return truncate(summary, MAX_QUERY_MESSAGE_LENGTH)), so wrapping the INSERT branch in the same helper keeps the two paths consistent — which is the point of this fix (the INSERT summary was previously unbounded at ~112 chars).

Making the INSERT result strictly <= MAX while the SELECT path stays at MAX + 1 would introduce an inconsistency. The clean way to get a hard <= MAX guarantee is to fix truncate itself (e.g. half_length = (length - 1) // 2), but that changes every even-length caller (10, 80) and their existing snapshots, which is out of scope for "bound the INSERT summary." Happy to do that as a follow-up if you'd prefer the strict bound.

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

_{Re-trigger cubic}

[chuenchen309]

Putting a number on the "out of scope" claim I made above, since it was an assertion and this is checkable:

Applying the suggested truncate change (reserving one character for …, i.e. half_length = (length - 1) // 2) fails 9 of the 24 tests in tests/test_db_statement_summary.py — test_long_select, test_one_join_long and friends, all of which pin exact 81-character SELECT output that predates this PR.

That's the reason this PR leaves truncate alone: the 81-vs-80 is a property of truncate itself (f'{s[:length // 2]}…{s[-(length // 2):]}' → 2 * (length // 2) + 1), shared by the SELECT path that already calls it. This PR only makes the INSERT branch honour the same bound the SELECT branch already honours; tightening truncate would change every existing SELECT summary and is a separate, deliberate decision.

Happy to send that as its own PR if the team wants the bound to be exact — it'd need the expected literals in those 9 tests updated too.