Mark ambiguous run summary recommendations by snoopuppy582 · Pull Request #1926 · pydantic/logfire

snoopuppy582 · 2026-05-13T20:00:41Z

Fixes #1296

Summary

Mark ambiguous logfire run recommendations (requests, sqlite3, urllib) with *.
Add a short note explaining that these packages can be detected from environment availability even when the app does not use them directly.
Keep normal recommendation/install-command behavior unchanged.

Tests

uv run pytest tests/test_cli.py::test_get_recommendation_texts tests/test_cli.py::test_get_recommendation_texts_marks_ambiguous_packages -q
uv run ruff check logfire/_internal/cli/run.py tests/test_cli.py
uv run ruff format --check logfire/_internal/cli/run.py tests/test_cli.py

Notes

Fresh narrow follow-up after Improve logfire run instrumentation recommendations #1922, scoped to the special-case wording requested in review.

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.

snoopuppy582 · 2026-05-13T20:24:26Z

I pushed a small snapshot-spacing follow-up after CI.

The test matrix was failing in tests/test_cli.py::test_inspect; the rendered Rich panel line for the new urllib* note had one fewer padding space than my snapshot expected.

Local checks after the follow-up:

uv run pytest tests/test_cli.py::test_get_recommendation_texts tests/test_cli.py::test_get_recommendation_texts_marks_ambiguous_packages -q -> 2 passed
uv run ruff check logfire/_internal/cli/run.py tests/test_cli.py -> passed
uv run ruff format --check logfire/_internal/cli/run.py tests/test_cli.py -> passed
git diff --check -> passed

snoopuppy582 · 2026-05-13T20:37:17Z

CI update after the snapshot-spacing follow-up:

lint, docs, test on Pyodide, Cubic review, and the Python 3.10-3.14 matrix jobs passed.
The remaining real test failure is only test on Python 3.9, pydantic 2, otel 1, in tests/otel_integrations/test_celery.py::test_instrument_celery.
That job reports 1586 passed, 41 skipped, 2 xfailed, and one Celery span assertion failure where the observed span is run/tasks.say_hello rather than the expected apply_async/tasks.say_hello.

This looks unrelated to this PR's CLI recommendation text/snapshot change. I tried to rerun the failed job, but GitHub requires repository admin rights for reruns on this workflow.

codecov · 2026-05-14T16:13:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

snoopuppy582 · 2026-05-15T11:28:24Z

Thanks, applied the wording suggestion in 450535bc.

Changes:

Changed ambiguous recommendation markers from urllib* to urllib [*].
Updated the note wording to: [*] ... may not actually be used by your app, in which case you can ignore ....
Updated the focused recommendation-text assertions and the inspect snapshot.

Verified locally:

uv run pytest tests/test_cli.py::test_get_recommendation_texts tests/test_cli.py::test_get_recommendation_texts_marks_ambiguous_packages -q -> 2 passed
uv run ruff check logfire/_internal/cli/run.py tests/test_cli.py -> passed
uv run ruff format --check logfire/_internal/cli/run.py tests/test_cli.py -> passed
git diff --check -> passed

Note: tests/test_cli.py::test_inspect still has the known Windows local Rich/box-drawing snapshot rendering difference, so I kept the repository snapshot aligned with the GitHub/reviewer suggestion rather than the local Windows mojibake output.

devin-ai-integration

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

coderabbitai · 2026-06-18T22:25:59Z

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: 43bfc280-b7c2-47bf-88cc-fd52ea32f93f

📥 Commits

Reviewing files that changed from the base of the PR and between 1b982a7 and 73690bc.

📒 Files selected for processing (2)

logfire/_internal/cli/run.py
tests/test_cli.py

🔗 Linked repositories identified

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

pydantic/pydantic-ai (auto-detected)
pydantic/pydantic (auto-detected)

📝 Walkthrough

Walkthrough

The CLI recommendation text generator in logfire/_internal/cli/run.py is updated to handle packages that are always present (stdlib or bundled with OpenTelemetry) but may not be used by the app. A new AMBIGUOUS_RECOMMENDATION_PACKAGES set identifies these packages; get_recommendation_texts() appends [*] to their checklist entries and adds an explanatory note using the new _format_package_list() helper. Tests update the test_inspect snapshot for urllib and add a dedicated test for the ambiguous-annotation logic.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main change: marking ambiguous recommendations in logfire run output with visual indicators.
Description check	✅ Passed	The description accurately explains the changes, references the linked issue `#1296`, and details the implementation approach and testing performed.
Linked Issues check	✅ Passed	The PR successfully implements the core requirement from `#1296` by marking ambiguous packages (requests, sqlite3, urllib) with [*] and adding an explanatory note about environment detection.
Out of Scope Changes check	✅ Passed	All changes are scoped to handling ambiguous recommendations: a new constant, helper function, test updates, and snapshot changes—no unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

alexmojaki · 2026-06-19T14:08:15Z

Thanks!

Mark ambiguous run summary recommendations

8f3719c

cubic-dev-ai Bot reviewed May 13, 2026

View reviewed changes

test: match run summary snapshot spacing

a0642f4

alexmojaki reviewed May 14, 2026

View reviewed changes

Comment thread tests/test_cli.py Resolved

test: apply urllib recommendation wording suggestion

450535b

Merge branch 'main' into fix/run-summary-special-case-note

913ff45

devin-ai-integration Bot reviewed May 19, 2026

View reviewed changes

Comment thread logfire/_internal/cli/run.py Resolved

test: fix ambiguous package list grammar

73690bc

coderabbitai Bot approved these changes Jun 18, 2026

View reviewed changes

Merge branch 'main' into fix/run-summary-special-case-note

2255602

alexmojaki enabled auto-merge (squash) June 19, 2026 14:08

alexmojaki merged commit 3b08489 into pydantic:main Jun 19, 2026
16 checks passed

alexmojaki mentioned this pull request Aug 18, 2026

logfire inspect suggests irrelevant packages #330

Closed

Navigation Menu

Repository navigation

Mark ambiguous run summary recommendations - #1926

Mark ambiguous run summary recommendations#1926
alexmojaki merged 6 commits into
pydantic:mainfrom
snoopuppy582:fix/run-summary-special-case-note

snoopuppy582 commented May 13, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

snoopuppy582 commented May 13, 2026

Uh oh!

snoopuppy582 commented May 13, 2026

Uh oh!

codecov Bot commented May 14, 2026

Uh oh!

Uh oh!

snoopuppy582 commented May 15, 2026

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 18, 2026

Walkthrough

Uh oh!

alexmojaki commented Jun 19, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

snoopuppy582 commented May 13, 2026

Summary

Tests

Notes

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

snoopuppy582 commented May 13, 2026

Uh oh!

snoopuppy582 commented May 13, 2026

Uh oh!

codecov Bot commented May 14, 2026

Codecov Report

Uh oh!

Uh oh!

snoopuppy582 commented May 15, 2026

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 18, 2026

Walkthrough

Uh oh!

alexmojaki commented Jun 19, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants