Skip to content

Commit 90f1673

Browse files
authored
feat: add CodeRabbit review instructions for account module (hiero-ledger#1883)
Signed-off-by: MonaaEid <monaa_eid@hotmail.com>
1 parent e74a5dd commit 90f1673

File tree

2 files changed

+101
-1
lines changed

2 files changed

+101
-1
lines changed

.coderabbit.yaml

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1816,7 +1816,7 @@ reviews:
18161816
- Flag the **missing override** in `TopicMessageSubmitTransaction` as **MAJOR**
18171817
if the base-class default is not appropriate for message submission, or add a
18181818
comment confirming the base-class value is intentional.
1819-
- Flag any value that deviates from the table above without a comment as **MAJOR**.
1819+
- Flag any value that deviates from sibling-class defaults as **MAJOR**.
18201820
18211821
#### 5c) General fee/limit logic
18221822
- Confirm any fee/max payment logic is consistent and does not allow unintended
@@ -1850,6 +1850,102 @@ reviews:
18501850
- path: "src/hiero_sdk_python/crypto/**/*.py"
18511851
instructions: *crypto_review_instructions
18521852

1853+
- path: "src/hiero_sdk_python/account/**/*.py"
1854+
instructions: |
1855+
You are reviewing Python files in the `src/hiero_sdk_python/account/` directory.
1856+
These files implement account-related transactions, queries, and data types for the Hiero SDK.
1857+
1858+
> Note: Protobuf schema alignment (field names, types, oneof, bytes normalization,
1859+
> repeated defaults, and canonical `.proto` URL derivation) is already enforced by
1860+
> the global `src/hiero_sdk_python/**/*.py` instruction. Do NOT re-run those checks
1861+
> here — focus exclusively on the account-domain concerns below.
1862+
1863+
---
1864+
1865+
## Review priorities (highest → lowest)
1866+
1867+
### 1) Protobuf correctness (must-not-break)
1868+
- Validate field names, field types, and field numbers +
1869+
repeated/optional semantics against the canonical `.proto` schema.
1870+
- Ensure `_to_proto` / `_from_proto` set the correct oneof branches
1871+
and do not drop fields.
1872+
- Treat silent coercions, defaulting, or ignored exceptions as
1873+
**BLOCKER** if they could change what is signed or sent.
1874+
1875+
### 2) AccountId — alias / EVM address handling
1876+
`AccountId` has three identity modes: numeric `num`, `alias_key`,
1877+
and `evm_address`. Verify `_from_proto`/`_to_proto` symmetry across
1878+
all three. `__hash__` only covers `(shard, realm, num)` — alias-only
1879+
IDs with `num=0` can collide in dicts/sets. **MAJOR** if broken.
1880+
1881+
### 3) Allowance mutation paths
1882+
When modifying existing `TokenNftAllowance` entries, mutation path and
1883+
append path must set `approved_for_all` to the same intended value.
1884+
`None` passed to typed arguments like `amount` must raise, not silently
1885+
produce zero. **MAJOR**.
1886+
1887+
### 4) Errors & API ergonomics
1888+
- Errors should be actionable and consistent; avoid swallowing or
1889+
printing RPC/precheck errors.
1890+
- Ambiguous exceptions or inconsistent return types are **MINOR** /
1891+
**MAJOR** depending on impact.
1892+
1893+
---
1894+
1895+
## ARCHITECTURAL INVARIANTS (stable)
1896+
These describe what must always be true, regardless of code changes.
1897+
They are ordered by severity.
1898+
1899+
1. **Falsy-guard trap on optional integers** — Fields like
1900+
`staked_node_id` where `0` is a valid value must never use
1901+
`if field:` guards. Use `if field is not None:`. **BLOCKER**
1902+
if it silently drops valid data.
1903+
1904+
2. **Mutable default arguments** — Flag any mutable object
1905+
(`list`, `dict`, `set`, or non-frozen class instance) used as
1906+
a default parameter in `__init__`. Default to `None` and assign
1907+
in the body. **MAJOR**.
1908+
1909+
Exception: frozen/immutable types are safe as defaults and must
1910+
NOT be flagged. This includes `@dataclass(frozen=True)` classes
1911+
(e.g., `Duration`), tuples, frozensets, and primitives (int, str,
1912+
bool, None).
1913+
1914+
3. **Library-safe error handling** — No bare `except Exception`
1915+
with `print`/`traceback` in library code. Callers cannot
1916+
suppress stdout side effects. **MAJOR**.
1917+
1918+
---
1919+
1920+
## KNOWN INSTANCES (transient — remove when fixed)
1921+
1922+
_Check if these still exist before flagging._
1923+
1924+
- `AccountRecordsQuery._make_request`: bare except + print
1925+
→ violates invariant #3
1926+
- `AccountId.from_string`: verify `from e` chaining not removed
1927+
→ exception context silently lost if removed
1928+
- `AccountInfo._to_proto`: falsy guard on `staked_node_id`
1929+
→ violates invariant #1
1930+
1931+
---
1932+
1933+
## Test expectations
1934+
PRs modifying account code should have tests covering:
1935+
- AccountId round-trip across all three identity modes
1936+
- `__hash__` behavior for alias-only IDs with `num=0`
1937+
- Invalid/None arguments raising errors, not silently defaulting
1938+
- `_from_proto(_to_proto(x))` field round-trip equality
1939+
1940+
---
1941+
1942+
## Severity labels
1943+
Use exactly these labels in all findings:
1944+
- **BLOCKER** — must be fixed before merge; wire-format, signing, or data-loss risk
1945+
- **MAJOR** — should be fixed; correctness or significant usability issue
1946+
- **MINOR** — should be fixed; low-risk correctness or ergonomics issue
1947+
- **NIT** — optional; style, naming, or documentation only
1948+
18531949
chat:
18541950
art: false # Don't draw ASCII art (false)
18551951
auto_reply: false # Don't allow bot to converse (spammy)

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](https://semver.org).
55
This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
66

77
## [Unreleased]
8+
9+
### Added
10+
- Added CodeRabbit review instructions in `.coderabbit.yaml` for account module `src/hiero_sdk_python/account/`.
11+
812
### Changed
913
- Changed pytest version to "pytest>=8.3.4,<10" (#1917)
1014

0 commit comments

Comments
 (0)