Skip to content

Expose ResolvedVariable reason#1945

Merged
alexmojaki merged 2 commits into
mainfrom
codex/resolved-variable-reason
May 21, 2026
Merged

Expose ResolvedVariable reason#1945
alexmojaki merged 2 commits into
mainfrom
codex/resolved-variable-reason

Conversation

@alexmojaki
Copy link
Copy Markdown
Collaborator

Summary

  • expose ResolutionReason and make ResolvedVariable.reason public
  • include code_default as the final resolution reason when a variable falls back to its code default
  • rename internal _reason usage to reason across variable providers and tests

Extracted from #1731 without the unrelated variable composition changes.

Tests

  • uv run pytest tests/test_variables.py
  • uv run ruff check logfire/variables/init.py logfire/variables/abstract.py logfire/variables/config.py logfire/variables/remote.py logfire/variables/variable.py tests/test_variables.py
  • uv run pyright logfire/variables/abstract.py logfire/variables/config.py logfire/variables/remote.py logfire/variables/variable.py tests/test_variables.py

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 21, 2026

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8e159cc
Status: ✅  Deploy successful!
Preview URL: https://7ed77882.logfire-docs.pages.dev
Branch Preview URL: https://codex-resolved-variable-reas.logfire-docs.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

Confidence score: 5/5

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

Re-trigger cubic

@alexmojaki alexmojaki force-pushed the codex/resolved-variable-reason branch from c0ce570 to 338239f Compare May 21, 2026 11:40
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread logfire/variables/variable.py
Comment on lines +294 to +295
label=serialized_result.label,
version=serialized_result.version,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

so was the missing label/version just a bug in main?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(AI) Yes, that looks like a pre-existing bug in main: the validation-error path already fell back to the code default while preserving the exception, but it rebuilt ResolvedVariable without carrying over the selected provider label/version. This PR fixes that while doing the _reason -> reason rename.

@alexmojaki alexmojaki force-pushed the codex/resolved-variable-reason branch from 2ad2395 to bbb9f94 Compare May 21, 2026 12:42
@alexmojaki alexmojaki enabled auto-merge (squash) May 21, 2026 15:57
@alexmojaki alexmojaki merged commit 1147a16 into main May 21, 2026
15 checks passed
@alexmojaki alexmojaki deleted the codex/resolved-variable-reason branch May 21, 2026 16:05
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.

1 participant