Refactor filesystem_size_quartet method

[mulkieran]

Related #1225

[coderabbitai]

Walkthrough

Type annotations were added across formatting and filesystem listing helpers. Function signatures were tightened, return types specified, and internal logic adjusted to handle None values explicitly. Table rendering helpers now use typed parameters. A new, typed filesystem_size_quartet helper was introduced and invoked with explicitly constructed Range values.

Changes

Cohort / File(s)	Summary of changes
Formatting utilities typing and logic src/stratis_cli/_actions/_formatting.py	Added explicit type hints to multiple functions (size_triple, get_property, _get_column_len, _print_row, print_table, get_uuid_formatter). Simplified size/free handling to rely on used. Standardized typed interfaces for rows/columns/file-like objects. Specified return types (e.g., Callable for UUID formatter).
Filesystem list typing and helper extraction src/stratis_cli/_actions/_list_filesystem.py	Introduced typed helper filesystem_size_quartet(total: Range, used: Optional[Range], limit: Optional[Range]) -> str inside Table.display. Updated call site to pass explicitly constructed Range/get_property-derived values rather than dbus_props access.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as CLI Command
  participant Table as Table.display
  participant FS as filesystem_size_quartet
  participant FMT as formatting.size_triple/print_table

  User->>CLI: run list-filesystem
  CLI->>Table: display()
  Note over Table: Build rows with typed values<br/>total: Range, used?: Range, limit?: Range
  Table->>FS: format quartet(total, used?, limit?)
  FS-->>Table: "total / used / free / limit" string
  Table->>FMT: print_table(headings, rows, alignment, file)
  FMT-->>User: rendered table

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A whisker twitch, a typing spree,
We labeled bytes with clarity.
Ranges hop, None’s kept in sight,
Tables line up, columns right.
With quartet songs and UUID flair—
I thump my foot: the types are there! 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title highlights only the refactoring of the filesystem_size_quartet method but omits the substantial addition of explicit type annotations and signature updates in the formatting helpers file, which represent a significant part of the changeset. Because it does not fully summarize the main scope of work, it fails to clearly communicate all of the primary updates in this pull request.	Please update the title to encompass both the refactoring of filesystem_size_quartet and the addition of explicit type annotations in the formatting helpers, for example “Add type annotations to formatting helpers and refactor filesystem_size_quartet method.”

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

• Public repositories are always opted into early access features.
• You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ff4e89 and 77f5e2c.

📒 Files selected for processing (2)

• src/stratis_cli/_actions/_formatting.py (7 hunks)
• src/stratis_cli/_actions/_list_filesystem.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/stratis_cli/_actions/_list_filesystem.py (1)

src/stratis_cli/_actions/_formatting.py (2)

• size_triple (39-56)
• get_property (59-73)

src/stratis_cli/_actions/_formatting.py (1)

src/stratis_cli/_actions/_physical.py (1)

• size (103-114)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test-runs (3.13)
• GitHub Check: test-runs (3.12)

🔇 Additional comments (10)

src/stratis_cli/_actions/_list_filesystem.py (2)

20-20: LGTM: Import addition supports new type annotations.

The addition of Optional to the typing imports is necessary for the new type annotations in the filesystem_size_quartet function signature.

---

127-136: LGTM: Function signature properly typed.

The function signature now explicitly declares parameter and return types, improving type safety and code clarity. The implementation logic remains unchanged—it still uses size_triple and formats the limit appropriately.

src/stratis_cli/_actions/_formatting.py (8)

20-20: LGTM: Import additions support new type annotations.

The typing imports (Callable, List, Optional) are necessary for the type annotations added throughout this file.

---

24-25: LGTM: D-Bus and justbytes imports support type annotations.

The Struct import from dbus and Range import from justbytes are needed for the explicit type annotations in get_property and size_triple respectively.

---

39-56: LGTM: Type annotations improve clarity.

The addition of explicit type annotations to size_triple makes the function signature clearer. The logic correctly handles the case where used is None by setting free to None as well, which then displays as TABLE_FAILURE_STRING in the formatted output.

---

59-73: LGTM: Type annotations align with D-Bus property handling.

The type annotations correctly reflect that this function handles D-Bus Struct types (representing optional properties as (bool, T) pairs) and returns either the transformed value or the default.

---

76-97: LGTM: Type annotations clarify integer parameters.

The explicit type annotations make it clear that this function operates on integer measurements of column widths and character counts.

---

100-127: LGTM: Type annotations improve function signature clarity.

The type annotations for _print_row make the expected parameter types explicit. Using Any for the file parameter is appropriate since it accepts any file-like object with a write method.

---

129-171: LGTM: Type annotations enhance table printing interface.

The type annotations clearly document the expected structure of the table data. The use of List[str] for column headings and alignments, and List[Any] for row entries is appropriate given the flexible nature of table contents.

---

173-183: LGTM: Return type annotation documents formatter function.

The return type Callable clearly indicates this function returns a formatting function, improving the API documentation.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify that mofilesystem.Size() returns a value compatible with Range constructor.

The refactored call site now explicitly constructs Range(mofilesystem.Size()) instead of relying on implicit conversions. Ensure that mofilesystem.Size() returns a numeric value that the Range constructor accepts (likely an integer representing bytes).

---

🏁 Script executed:

#!/bin/bash
# Description: Verify the type signature of mofilesystem.Size() and its compatibility with Range constructor

# Search for MOFilesystem class and Size method definition
ast-grep --pattern $'class MOFilesystem {
  $$$
  Size($$$) {
    $$$
  }
  $$$
}'

# Also search for Size method calls to understand return type
rg -nP --type=py -A3 'def Size\(' 

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Locate MOFilesystem class definition
rg -nP 'class\s+MOFilesystem' -C3 -g '*.py'
# Locate any Size method definitions
rg -nP 'def\s+Size\(' -C3 -g '*.py'
# Locate Range class constructor
rg -nP 'class\s+Range' -C3 -g '*.py'
# Print top of list_filesystem action file for imports context
sed -n '1,50p' src/stratis_cli/_actions/_list_filesystem.py

Length of output: 1606

---

🏁 Script executed:

#!/bin/bash
# Print first part of the data module to locate MOFilesystem and its Size method
sed -n '1,200p' src/stratis_cli/_actions/_data.py
# Specifically search for the Size method in the data module
rg -nP 'def\s+Size\(' -C2 -g 'src/stratis_cli/_actions/_data.py'

# Inspect justbytes.Range constructor signature if installed
python3 - << 'EOF'
import inspect
try:
    from justbytes import Range
    print("Range signature:", inspect.signature(Range))
except Exception as e:
    print("Error importing or inspecting Range:", e)
EOF

Length of output: 5491

---

🏁 Script executed:

#!/bin/bash
# Locate interface spec for Filesystem in introspect module
grep -R 'org.storage.stratis3.Filesystem' -n src/stratis_cli/_actions/_introspect.py
# Print context around the Filesystem interface definition
sed -n '100,300p' src/stratis_cli/_actions/_introspect.py

Length of output: 8492

---

Convert MOFilesystem.Size() to int before passing to Range. The D-Bus Size property is defined as a string (type="s"), so MOFilesystem.Size() returns a str. Use Range(int(mofilesystem.Size())) to ensure the constructor receives a numeric value.

🤖 Prompt for AI Agents

In src/stratis_cli/_actions/_list_filesystem.py around lines 142 to 146, the
call passes MOFilesystem.Size() (a D-Bus string) into Range which expects a
numeric value; convert it to an int before constructing Range by calling
Range(int(mofilesystem.Size())), keeping the other get_property calls unchanged
so the filesystem_size_quartet receives a numeric Size.