Fix: Multi version bootstrap age filter fallback for edge cases

[rd4398]

Commit 1:

feat(resolver): add AgeFallback.NEWEST for multi-version age filter
Replace the boolean `fallback_on_empty_age_filter` parameter in
`find_all_matching_from_provider()` with an `AgeFallback` enum that
supports three strategies when max-release-age filtering removes all
candidates:

- `ALL`: keep every candidate (single-version default, unchanged)
- `NEWEST`: keep only the single newest candidate (new)
- `NONE`: return empty list

Multi-version mode now uses `AgeFallback.NEWEST` so that when a
dependency has no releases within the age window (e.g. bar from 2
years ago depended on by recent foo), the newest version is built
instead of failing. This prevents cascading failures where the
dependent package also fails to build.

Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>

Commit 2:

fix(resolver): constrained packages bypass max-release-age filtering
Packages with explicit constraints (e.g. `boto3==1.35.88` in
constraints files) now skip age filtering entirely in
`find_all_matching_from_provider()`. A constraint is explicit user
intent and should not be silently overridden by the age heuristic.

Previously in multi-version mode with `--max-release-age`, a
constrained package whose pinned version was older than the age
window would have all candidates removed, causing silent resolution
failure. In single-version mode the fallback kept all candidates
anyway, but emitted a misleading "keeping all to avoid empty
resolution" warning.

Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>

Closes #1288

[coderabbitai]

Warning

Review limit reached

@rd4398, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0f7b64a5-9ba9-4395-a545-0b07f8920a6a

📥 Commits

Reviewing files that changed from the base of the PR and between 925a19e and 6434bda.

📒 Files selected for processing (7)

• e2e/ci_bootstrap_suite.sh
• e2e/test_bootstrap_age_constraint_bypass.sh
• e2e/test_bootstrap_age_fallback_newest.sh
• src/fromager/bootstrap_requirement_resolver.py
• src/fromager/resolver.py
• tests/test_bootstrap_requirement_resolver.py
• tests/test_cooldown.py

📝 Walkthrough

Walkthrough

The resolver now uses the AgeFallback enum for age-filter behavior. Constrained packages bypass --max-release-age filtering. Multi-version bootstrap falls back to the newest candidate when all releases are too old. Single-version bootstrap retains all candidates. Unit and end-to-end tests cover the new strategies and CI execution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the multi-version bootstrap age-filter fallback change.
Description check	✅ Passed	The description explains the AgeFallback changes and the constrained-package behavior addressed by the pull request.
Linked Issues check	✅ Passed	The implementation satisfies issue #1288 by selecting the newest unconstrained version and bypassing age filtering for constrained packages.
Out of Scope Changes check	✅ Passed	The source changes and tests directly support the age-filter fallback and constrained-package requirements in issue #1288.

---

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.}

[coderabbitai]

🧹 Nitpick comments (2)

src/fromager/resolver.py (1)

304-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add requirement log context.

These logs describe one requirement but do not use req_ctxvar_context(). Wrap the age-filter operation in the established requirement context before logging. As per coding guidelines, “Use req_ctxvar_context() for per-requirement logging.”

Also applies to: 346-357

🤖 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 `@src/fromager/resolver.py` around lines 304 - 313, Wrap the age-filter logging
paths in the established req_ctxvar_context() requirement context before
emitting messages. Update the branches around the constrained-package log and
the corresponding age-filter operation near the later referenced block,
preserving the existing filtering behavior while ensuring all per-requirement
logs include the request context.

Source: Coding guidelines

tests/test_cooldown.py (1)

807-828: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove log-message assertions from these behavior tests.

The result and wheel assertions already verify the required behavior. Log text is diagnostic output and can change without changing behavior.

• tests/test_cooldown.py#L807-L828: remove caplog assertions after verifying the empty result.
• tests/test_cooldown.py#L831-L853: remove caplog assertions after verifying version 2.0.0.
• tests/test_cooldown.py#L1008-L1036: remove caplog assertions after verifying constrained version 1.2.2.
• tests/test_cooldown.py#L1039-L1060: remove the negative log assertion after verifying the empty result.
• e2e/test_bootstrap_age_constraint_bypass.sh#L59-L67: remove the log grep; retain the wheel assertion.
• e2e/test_bootstrap_age_fallback_newest.sh#L51-L59: remove the log grep; retain the wheel-count and version assertions.

Based on learnings, “avoid asserting on exact log output strings since they are brittle implementation details.”

🤖 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 `@tests/test_cooldown.py` around lines 807 - 828, Remove brittle log-output
assertions while preserving all behavior assertions: in tests/test_cooldown.py
ranges 807-828, 831-853, 1008-1036, and 1039-1060, delete the caplog assertions
after the existing result/version checks; in
e2e/test_bootstrap_age_constraint_bypass.sh range 59-67 and
e2e/test_bootstrap_age_fallback_newest.sh range 51-59, remove only the log grep
commands and retain the wheel, count, and version assertions.

Source: Learnings

🤖 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.

Nitpick comments:
In `@src/fromager/resolver.py`:
- Around line 304-313: Wrap the age-filter logging paths in the established
req_ctxvar_context() requirement context before emitting messages. Update the
branches around the constrained-package log and the corresponding age-filter
operation near the later referenced block, preserving the existing filtering
behavior while ensuring all per-requirement logs include the request context.

In `@tests/test_cooldown.py`:
- Around line 807-828: Remove brittle log-output assertions while preserving all
behavior assertions: in tests/test_cooldown.py ranges 807-828, 831-853,
1008-1036, and 1039-1060, delete the caplog assertions after the existing
result/version checks; in e2e/test_bootstrap_age_constraint_bypass.sh range
59-67 and e2e/test_bootstrap_age_fallback_newest.sh range 51-59, remove only the
log grep commands and retain the wheel, count, and version assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d6f59d0e-48a2-4967-9d79-35f7577e6234

📥 Commits

Reviewing files that changed from the base of the PR and between e979af8 and 3dd9464.

📒 Files selected for processing (7)

• e2e/ci_bootstrap_suite.sh
• e2e/test_bootstrap_age_constraint_bypass.sh
• e2e/test_bootstrap_age_fallback_newest.sh
• src/fromager/bootstrap_requirement_resolver.py
• src/fromager/resolver.py
• tests/test_bootstrap_requirement_resolver.py
• tests/test_cooldown.py

[andre-motta]

LGTM. Lets test this with https://pypi.org/project/dead/ on walkerpass or similar before releasing!

[rd4398]

The fix for CI failures are part of #1287
I will rebase the PR once above mentioned PR lands

[LalatenduMohanty]

@rd4398 #1287 has merged

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[pavank63]

LGTM

Tested on local system (x86_64) with old (main: 0.92.1.dev13) and new (PR: 0.92.1.dev15) side-by-side using --multiple-versions.

tomli>=2.0,<=2.0.2 with --max-release-age=1

Scenario	OLD (main)	NEW (PR)
Unconstrained	all 3 candidate(s) older than 1 days → empty, skipped	falling back to newest version 2.0.2 → resolves
Constrained tomli==2.0.0	all 1 candidate(s) older than 1 days → pinned version skipped	skipping age filter for constrained package → resolves 2.0.0

dead>=1.0 (latest 2.1.0 uploaded 2025-02-08, ~543 days old)

Scenario	OLD (main)	NEW (PR)
--max-release-age=30	all 11 candidate(s) older than 30 days → empty, skipped	falling back to newest version 2.1.0 → resolves
--max-release-age=365	all 11 candidate(s) older than 365 days → empty, skipped	falling back to newest version 2.1.0 → resolves
--max-release-age=600	have 3 candidate(s) within 600 days → resolves 3	have 3 candidate(s) within 600 days → resolves 3
--max-release-age=30 + dead==1.5.2	all 1 candidate(s) older than 30 days → pinned version skipped	skipping age filter for constrained package → resolves 1.5.2

When versions fall within the age window, both behave identically. The PR only changes behavior when all candidates are outside the window.

[LalatenduMohanty]

I have approved the PR however my concern is that multi-version resolution now has three fallback layers stacked:

1. Age filter with NEWEST fallback (new)
2. Cache server fallback (pre-existing, only when cache URL configured)
3. Log warning and return empty (pre-existing)

That's a lot of "try this, else try that" for one resolution path, and it makes the behavior hard to predict for someone reading the code cold.

Single-version mode has none of this . The two modes are diverging in resolution semantics, which could become a maintenance burden as more edge cases surface.

I will create a follow up issue which we can look in to resolving this post this PR merge.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request