Skip to content

refactor: make_hires_sigma_schedule() simplification - #2244

Merged
leejet merged 2 commits into
leejet:masterfrom
vmobilis:patch-1
Oct 29, 2026
Merged

leejet merged 2 commits into
leejet:masterfrom
vmobilis:patch-1

Conversation

@vmobilis

@vmobilis vmobilis commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After PR #2227, the make_hires_sigma_schedule() function can be simplified:

  1. effective_steps and scheduler_steps are cropped to 1...INT_MAX range (request.cpp#L507).
  2. Therefore t_enc = effective_steps - 1 is in range 0...INT_MAX - 1.
    Therefore t_enc is a positive integer and is compared with positive integers at request.cpp#L523, then (request.cpp#L522_L525):
size_t t_enc = static_cast<size_t>(effective_steps - 1);
if (t_enc >= static_cast<size_t>(scheduler_steps)) {
    t_enc = static_cast<size_t>(scheduler_steps) - 1;
}

can be simplified to:
t_enc = std::min(effective_steps - 1, scheduler_steps - 1);
==
t_enc = std::min(effective_steps, scheduler_steps) - 1;

  1. Therefore:
    - t_enc - 1
    ==
    - std::min(effective_steps, scheduler_steps)
    (request.cpp#L529)

Checklist

Summary by CodeRabbit

  • Bug Fixes
    • High-resolution generation now selects the appropriate final steps from the available schedule, including when the requested step count differs from the scheduler’s available steps. This improves consistency across step-count settings.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3362f38f-a21e-4e03-8ee2-bcce081232ec
Commits

Reviewing files that changed from the base of the PR and between 89c5e52 and fe7de1a.

Files selected for processing: 1
  • src/pipeline/request.cpp

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

make_hires_sigma_schedule now calculates the sigma suffix start directly from the scheduler and effective step counts. The returned schedule remains the suffix beginning at that index.

Changes: Hires sigma schedule

Layer / File(s) Summary
Schedule suffix selection
src/pipeline/request.cpp
The suffix start index now uses max(0, scheduler_steps - effective_steps), replacing the t_enc calculation and clamp.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Suggested reviewers: leejet

Merge Risk: Minimal · up to fe7de

No actionable merge-blocking risk is introduced by this formula simplification.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "fix: make_hires_sigma_schedule() simplification" names the function the PR changes and states the kind of change (a simplification). The raw summary confirms the change replaces the expli…
Description check Passed The description has a Summary section that explains the change. It cites the prior PR #2227, the range constraints on effective_steps and scheduler_steps, and the simplified t_enc logic with links to …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix: make_hires_sigma_schedule() simplification" names the function the PR changes and states the kind of change (a simplification). The raw summary confirms the change replaces the explicit t_enc calculation and clamp with a start index computed as max(0, scheduler_steps - effective_steps). The title is short and specific. It does not name the mechanism, but it covers the main change.

Full details: Description check

Explanation

The description has a Summary section that explains the change. It cites the prior PR #2227, the range constraints on effective_steps and scheduler_steps, and the simplified t_enc logic with links to the code. The checklist is marked as confirmed. The template's Related Issue / Discussion section is missing. It links PR #2227 in the Summary, but it does not add it under that heading. The Additional Information section is also missing. It has no verification notes, sample output, or testing details.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Cppcheck (2.22.0)
src/pipeline/request.cpp

Cppcheck timed out; analysis of this file is incomplete

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

Comment @coderabbitai help to get the list of available commands.

@leejet
leejet merged commit 997478a into leejet:master Oct 29, 2026
11 checks passed
@vmobilis vmobilis changed the title fix: make_hires_sigma_schedule() simplification refactor: make_hires_sigma_schedule() simplification Nov 2, 2026
@vmobilis
vmobilis deleted the patch-1 branch November 2, 2026 06:47
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.

2 participants