Add total parameter to Progress widget #497
Conversation
WalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Progress
participant Data as Data Context
Caller->>Progress: init(field, width, total, filled, empty, when)
Note right of Progress#lightgreen: Store `total` on instance
Caller->>Progress: _render_text(data, manager)\npreview or runtime
alt Preview mode
Progress->>Progress: value = 15
else Runtime mode
Progress->>Data: get(field)
Data-->>Progress: value
end
Progress->>Progress: percent = (value / total) * 100
Progress-->>Caller: Rendered bar string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/aiogram_dialog/widgets/text/progress.py (2)
27-29: Preview semantics likely shifted; confirm intended behaviorWith
value = 15and new percent computed as(value / self.total) * 100, preview percent now depends ontotal. Previously preview was 15% (effectively), now it is15/total * 100. Iftotal != 100, preview no longer shows a stable 15% bar.If the goal is a consistent preview of 15%, compute value from
total.- if manager.is_preview(): - value = 15 + if manager.is_preview(): + # keep preview at a stable 15% regardless of total + value = self.total * 0.15
24-36: Add tests coveringtotal, clamping, and preview behaviorPlease add unit tests that verify:
- Backward compatibility for positional args when
totalis moved to the end.total <= 0raisesValueError.- Missing field defaults to 0% without raising.
- Negative
valueis rendered as positive percent (abs applied) and clamped to 100% for large values.- Preview displays a stable 15% (if you accept the preview refactor).
I can draft tests for these scenarios once you confirm the desired preview semantics and final constructor signature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/aiogram_dialog/widgets/text/progress.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/aiogram_dialog/widgets/text/progress.py (4)
src/aiogram_dialog/widgets/text/base.py (4)
_render_text(32-38)_render_text(74-77)_render_text(86-90)_render_text(128-135)src/aiogram_dialog/manager/manager.py (1)
is_preview(135-136)src/aiogram_dialog/tools/preview.py (1)
is_preview(153-154)src/aiogram_dialog/manager/sub_manager.py (1)
is_preview(65-66)
🔇 Additional comments (1)
src/aiogram_dialog/widgets/text/progress.py (1)
8-16: I’m running a quick grep sweep to confirm the constructor signature and existing usages ofProgress—I’ll follow up once we see where and how it’s instantiated to decide if we should movetotal.
| super().__init__(when) | ||
| self.field = field | ||
| self.width = width | ||
| self.total = total | ||
| self.filled = filled | ||
| self.empty = empty |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate total to prevent division-by-zero and invalid input
total can be 0 or negative, causing a ZeroDivisionError or nonsensical output. Add a guard.
self.field = field
self.width = width
- self.total = total
+ if total <= 0:
+ raise ValueError("Progress.total must be a positive number")
+ self.total = total
self.filled = filled
self.empty = empty📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| super().__init__(when) | |
| self.field = field | |
| self.width = width | |
| self.total = total | |
| self.filled = filled | |
| self.empty = empty | |
| super().__init__(when) | |
| self.field = field | |
| self.width = width | |
| if total <= 0: | |
| raise ValueError("Progress.total must be a positive number") | |
| self.total = total | |
| self.filled = filled | |
| self.empty = empty |
🤖 Prompt for AI Agents
In src/aiogram_dialog/widgets/text/progress.py around lines 17 to 22, the
constructor accepts a total that may be zero or negative which can lead to
ZeroDivisionError or invalid output; add an explicit guard that validates total
is a positive integer (e.g. if not isinstance(total, int) or total <= 0: raise
ValueError("total must be a positive integer")), and update any callers/tests
accordingly so the widget never proceeds with total <= 0.
| value = 15 | ||
| else: | ||
| percent = data.get(self.field) | ||
| value = data.get(self.field) | ||
| percent = (value / self.total) * 100 | ||
| done = round((self.width * percent) / 100) | ||
| rest = self.width - done |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Handle missing/non-numeric values and clamp percent to [0, 100]; apply abs() as per PR description
data.get(self.field)may returnNone, leading toTypeErroron division.- The PR description says “use absolute values”, but
abs()isn’t applied. - Large or negative values can produce bars longer than
widthor negative segments. Clamp percent to [0, 100] to keep the bar stable.
- else:
- value = data.get(self.field)
- percent = (value / self.total) * 100
- done = round((self.width * percent) / 100)
+ else:
+ raw = data.get(self.field, 0)
+ try:
+ value = float(raw)
+ except (TypeError, ValueError):
+ value = 0.0
+ percent = max(0.0, min(100.0, (abs(value) / self.total) * 100))
+ done = round((self.width * percent) / 100)
rest = self.width - doneNotes:
- This keeps rendering within
widthand aligns with the “absolute values” intent. - If you prefer integer-only arithmetic, we can cast early and keep the same behavior.
🏁 Script executed:
#!/bin/bash
# Verify percent calculation and value assignment in src/aiogram_dialog/widgets/text/progress.py
rg -n -C3 'percent\s*=' src/aiogram_dialog/widgets/text/progress.py
rg -n -C3 'value\s*=' src/aiogram_dialog/widgets/text/progress.pyLength of output: 701
🏁 Script executed:
sed -n '1,200p' src/aiogram_dialog/widgets/text/progress.pyLength of output: 1015
Ensure robust handling of progress values in Progress._render_text
The current implementation will raise a TypeError if data.get(self.field) returns None, and negative or oversized values can produce invalid bar lengths. Per the PR description, values should be treated absolutely and confined to the [0,100] range. Update the assignment and calculation as follows:
• File: src/aiogram_dialog/widgets/text/progress.py, method _render_text (around lines 28–33)
- if manager.is_preview():
- value = 15
- else:
- value = data.get(self.field)
- percent = (value / self.total) * 100
- done = round((self.width * percent) / 100)
- rest = self.width - done
+ if manager.is_preview():
+ value = 15.0
+ else:
+ raw = data.get(self.field, 0)
+ try:
+ value = float(raw)
+ except (TypeError, ValueError):
+ value = 0.0
+ # Use absolute value and clamp percent to [0,100] to avoid overflows
+ percent = max(0.0, min(100.0, (abs(value) / self.total) * 100))
+ done = round((self.width * percent) / 100)
+ rest = self.width - done• Fallback non-numeric or missing inputs to 0.0
• Apply abs() to match PR intent
• Clamp percent between 0.0 and 100.0 to guarantee bar stability
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| value = 15 | |
| else: | |
| percent = data.get(self.field) | |
| value = data.get(self.field) | |
| percent = (value / self.total) * 100 | |
| done = round((self.width * percent) / 100) | |
| rest = self.width - done | |
| if manager.is_preview(): | |
| value = 15.0 | |
| else: | |
| raw = data.get(self.field, 0) | |
| try: | |
| value = float(raw) | |
| except (TypeError, ValueError): | |
| value = 0.0 | |
| # Use absolute value and clamp percent to [0,100] to avoid overflows | |
| percent = max(0.0, min(100.0, (abs(value) / self.total) * 100)) | |
| done = round((self.width * percent) / 100) | |
| rest = self.width - done |
2 participants
Add
totalparameter toProgresswidget and update percentage calculation to use absolute values.Summary by CodeRabbit
New Features
Refactor