refactor(resolver): move age filter fallback to resolver with cache server lookup

[rd4398]

Move the two-step fallback logic from _phase_resolve() into
BootstrapRequirementResolver._resolve(). When age filtering removes
all candidates in multi-version mode, the resolver now queries the
remote wheel cache server for the newest available version and returns
only that single version. The resolver warns on zero results but does
not error — _phase_resolve() raises the exception.

Co-Authored-By: Claude claude@anthropic.com

Signed-off-by: Rohan Devasthale rdevasth@redhat.com

Fixes: #1162

[coderabbitai]

Warning

Rate limit exceeded

@rd4398 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 45 minutes and 40 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 753b7007-d64f-4976-b5d3-74bf9fb49099

📥 Commits

Reviewing files that changed from the base of the PR and between b49421a and a33acb9.

📒 Files selected for processing (6)

• src/fromager/bootstrap_requirement_resolver.py
• src/fromager/bootstrapper.py
• src/fromager/resolver.py
• tests/test_bootstrap_requirement_resolver.py
• tests/test_bootstrapper_iterative.py
• tests/test_cooldown.py

📝 Walkthrough

Walkthrough

find_all_matching_from_provider gained a fallback_on_empty_age_filter parameter to control behavior when max_age_cutoff removes all candidates. BootstrapRequirementResolver now accepts multiple_versions and cache_wheel_server_url, passes fallback_on_empty_age_filter=not multiple_versions during sdist resolution, caches only non-empty results, and will query a remote wheel-cache server via _resolve_from_cache_server when in multi-version mode and initial source resolution is empty. Bootstrapper forwards multiple_versions to the resolver. Tests added for cache-server fallback, age-filter behavior, and iterative empty-resolution handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes fully address issue #1162: the PR implements a two-step fallback that checks the wheel cache first and only re-resolves without age filter as a last resort, preventing resolution of all versions.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: resolver parameter additions, bootstrapper integration, fallback logic, and corresponding test coverage.
Title check	✅ Passed	The title accurately reflects the main change: moving age filter fallback logic to the resolver and adding cache server lookup capability.
Description check	✅ Passed	The PR description clearly explains the change: moving two-step fallback logic into BootstrapRequirementResolver._resolve() for age filter fallback in multi-version mode, with specific details about behavior and the linked issue.

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

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[rd4398]

I considered two approaches:

1. Move age filtering to the bootstrapper — Remove age checks from the resolver entirely and filter in the bootstrapper where we have cache access.

2. Two-step resolution in the bootstrapper (chosen) — Let the resolver signal "all candidates too old" by returning empty, then let the bootstrapper decide what to do based on cache state.

This PR implements approach 2 because

• The age filter is used in three call sites (resolver.resolve(), bootstrap_requirement_resolver._resolve(), sources.resolve_source()). Moving it out would require threading upload_time metadata through all return paths — today the resolver returns
  list[tuple[str, Version]], not full Candidate objects, so the bootstrapper has no way to inspect upload times without duplicating PyPI metadata lookups.
• Single-version mode still needs the fallback. The resolver.resolve() and sources.resolve_source() paths return results[0] directly — an empty list there would crash. The fallback_on_empty_age_filter flag (default True) preserves this safety for all existing
  callers while letting multi-version mode opt out.
• The bootstrapper already has the right context. _phase_resolve() already checks the wheel cache per-version. Adding a "do we have any cached wheel?" check before falling back is a natural extension of that existing pattern, not a new abstraction.

In short: the resolver stays a pure "find candidates" layer, and the bootstrapper — which owns the build/cache decisions — gets to make the smarter choice when no fresh candidates exist.

[dhellmann]

Why don't we include the age check here for pre-built wheels?

[dhellmann]

I think we're going to want to look at the remote cache server, not the local one. The first time we bootstrap a package that has not had a recent release we won't have the file locally but it might already have been built and put in the wheel server from a previous job.

[rd4398]

Yeah done in latest commit

[dhellmann]

Rohan and I spoke directly about some other changes to this PR to update the contract for the bootstrap resolver to be "tell me the list of versions to build for this rule" and to therefore move some of the new logic from this change out of the phase method and into the bootstrap resolver instead.

We agreed that if the "normal" resolution using the existing steps results in no versions, the resolver should fall back to looking at the remote wheel server (if any) and pick the newest version of the package available on that server that matches the rule, but to ignore the age and to only return 1 version, even in the mode where we are building multiple versions. That will ensure that the bootstrapped re-processes one version of the package to get its transitive dependencies again and keep those up to date, without re-processing all versions of the package when we know they are not new.

We also agreed that the bootstrap resolver should warn if it gets 0 results, but not report an error, and the phase method should be responsible for raising an exception because that is where we know that we will eventually fail the current build because we could not find any versions of a dependency anywhere.

@rd4398 please let me know if I forgot any details about what we said.

[rd4398]

Rohan and I spoke directly about some other changes to this PR to update the contract for the bootstrap resolver to be "tell me the list of versions to build for this rule" and to therefore move some of the new logic from this change out of the phase method and into the bootstrap resolver instead.

We agreed that if the "normal" resolution using the existing steps results in no versions, the resolver should fall back to looking at the remote wheel server (if any) and pick the newest version of the package available on that server that matches the rule, but to ignore the age and to only return 1 version, even in the mode where we are building multiple versions. That will ensure that the bootstrapped re-processes one version of the package to get its transitive dependencies again and keep those up to date, without re-processing all versions of the package when we know they are not new.

We also agreed that the bootstrap resolver should warn if it gets 0 results, but not report an error, and the phase method should be responsible for raising an exception because that is where we know that we will eventually fail the current build because we could not find any versions of a dependency anywhere.

@rd4398 please let me know if I forgot any details about what we said.

@dhellmann yeah that summarizes it! I have pushed changes in latest commit based on our discussion

[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 `@src/fromager/bootstrap_requirement_resolver.py`:
- Line 131: The code assumes results from _resolve() are non-empty and indexes
results[0], which can raise IndexError when _resolve() returns []; update the
caller (the function using return_all_versions and calling _resolve()) to first
check if results is empty and handle it explicitly—either return [] when
return_all_versions is true or when not, raise a clear resolution exception
(e.g., ResolutionError or ValueError) with a descriptive message—so replace the
direct [results[0]] indexing with an empty-check guard before returning.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 30c1092f-88ce-44be-af63-edab185d1043

📥 Commits

Reviewing files that changed from the base of the PR and between 699995e and b49421a.

📒 Files selected for processing (4)

• src/fromager/bootstrap_requirement_resolver.py
• src/fromager/bootstrapper.py
• tests/test_bootstrap_requirement_resolver.py
• tests/test_bootstrapper_iterative.py