Skip to content

fix(configs): make environment variable name suggestions deterministic#3142

Merged
hubcio merged 3 commits into
apache:masterfrom
Standing-Man:deterministic-env-var-suggestions
Apr 22, 2026
Merged

fix(configs): make environment variable name suggestions deterministic#3142
hubcio merged 3 commits into
apache:masterfrom
Standing-Man:deterministic-env-var-suggestions

Conversation

@Standing-Man
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #

Rationale

This PR makes environment variable suggestions deterministic in TypedEnvProvider.

When multiple candidate env vars have the same Levenshtein distance to an unknown variable, the previous implementation could return them in non-deterministic order (due to HashSet iteration). This change adds a stable tie-breaker so suggestions are consistently ordered.

What changed?

Sorting logic now:

  1. Sort by edit distance (ascending)
  2. Tie-break by variable name (lexicographical ascending)

Added test find_similar_vars_has_deterministic_order_for_same_distance that verifies stable output order for same-distance candidates.

Local Execution

  • Passed / not passed
    Passed
  • Pre-commit hooks ran / not ran
    ran

AI Usage

I used codex to check the code I wrote.

Signed-off-by: StandingMan <jmtangcs@gmail.com>
@Standing-Man Standing-Man changed the title fix: make environment variable name suggestions deterministic fix(core): make environment variable name suggestions deterministic Apr 20, 2026
@Standing-Man Standing-Man changed the title fix(core): make environment variable name suggestions deterministic fix(configs): make environment variable name suggestions deterministic Apr 20, 2026
@Standing-Man
Copy link
Copy Markdown
Contributor Author

Standing-Man commented Apr 20, 2026

Hi @hubcio and @spetz - Please review at your convinience.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.03%. Comparing base (54fe2bb) to head (3d46ed8).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3142       +/-   ##
=============================================
- Coverage     73.29%   56.03%   -17.26%     
  Complexity      943      943               
=============================================
  Files          1126     1124        -2     
  Lines         98435    88411    -10024     
  Branches      75608    65024    -10584     
=============================================
- Hits          72148    49542    -22606     
- Misses        23683    36288    +12605     
+ Partials       2604     2581       -23     
Components Coverage Δ
Rust Core 50.73% <100.00%> (-23.46%) ⬇️
Java SDK 62.30% <ø> (ø)
C# SDK 69.10% <ø> (-0.35%) ⬇️
Python SDK 81.43% <ø> (ø)
Node SDK 91.53% <ø> (+0.09%) ⬆️
Go SDK 40.83% <ø> (+1.41%) ⬆️
Files with missing lines Coverage Δ
...ore/configs/src/configs_impl/typed_env_provider.rs 90.47% <100.00%> (+0.21%) ⬆️

... and 278 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Standing-Man Standing-Man requested a review from numinnex April 21, 2026 13:44
@Standing-Man
Copy link
Copy Markdown
Contributor Author

Hi @numinnex, would you mind helping to trigger the CI when convenient?

@Standing-Man
Copy link
Copy Markdown
Contributor Author

Standing-Man commented Apr 22, 2026

Hi @hubcio, @numinnex, and @spetz — just checking if this PR is good to merge, or if there’s anything else I should address.
Thanks @numinnex for helping trigger the CI!

@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented Apr 22, 2026

hey, thanks for ping. PR looks good good. we're currently busy with release, so we merge PRs less often.

@Standing-Man
Copy link
Copy Markdown
Contributor Author

hey, thanks for ping. PR looks good good. we're currently busy with release, so we merge PRs less often.

Got it, thanks! No worries at all — happy to wait. I’m curious — do I need to @ a maintainer to help trigger the CI tests every time I submit a PR?

@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented Apr 22, 2026

@Standing-Man once your first PR gets merged, CI will be automatically run on the PRs you create. manual approval from maintainers is required only for first-time-contributors.

@hubcio hubcio merged commit f4222f0 into apache:master Apr 22, 2026
80 checks passed
@Standing-Man
Copy link
Copy Markdown
Contributor Author

@Standing-Man once your first PR gets merged, CI will be automatically run on the PRs you create. manual approval from maintainers is required only for first-time-contributors.

Thanks for your explanation, and I really appreciate all the maintainers’ reviews!

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.

4 participants