Skip to content

Commit 1d1e299

Browse files
authored
chore: apply CodeRabbit instructions for file module (hiero-ledger#1697) (hiero-ledger#1841)
Signed-off-by: riteshr19 <riteshrana36@gmail.com>
1 parent 8ebd3ba commit 1d1e299

2 files changed

Lines changed: 123 additions & 0 deletions

File tree

.coderabbit.yaml

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,122 @@ reviews:
671671
- Gas/value safe
672672
- Deterministic and user-protective
673673

674+
# FILE REVIEW INSTRUCTIONS
675+
file_review_instructions: &file_review_instructions |
676+
You are acting as a senior maintainer reviewing file service-related code
677+
in the hiero-sdk-python project.
678+
679+
This includes:
680+
- File transactions (FileCreateTransaction, FileUpdateTransaction, FileAppendTransaction, FileDeleteTransaction)
681+
- File query (FileContentsQuery, FileInfoQuery)
682+
- File data models (FileId, FileInfo)
683+
684+
NOTE:
685+
- Review focus levels (including labels like "HIGH SENSITIVITY") indicate where to apply extra scrutiny.
686+
- They do NOT map to production incident severity or operational urgency.
687+
- Only recommend fixes when behavior, safety, backwards compatibility, or chunking semantics are impacted.
688+
689+
Scope is STRICTLY LIMITED to:
690+
- Changes under src/hiero_sdk_python/file/
691+
- Their interaction with shared SDK base classes (Transaction, Query)
692+
693+
----------------------------------------------------------
694+
REVIEW FOCUS 1 — API STABILITY & BACKWARDS COMPATIBILITY
695+
(HIGH SENSITIVITY)
696+
----------------------------------------------------------
697+
Public API contracts for FileId, FileInfo, and file transactions are user-facing.
698+
699+
Verify that:
700+
- Setter method signatures (e.g., set_contents, set_file_memo) stay backward compatible.
701+
- Method chaining (returning self) is preserved.
702+
- FileId constructors and string representations don't break existing use cases.
703+
704+
If breaking changes are necessary, they must be explicit and deprecation warnings should be added.
705+
706+
----------------------------------------------------------
707+
REVIEW FOCUS 2 — CHUNKING SEMANTICS (HIGH VERIFICATION)
708+
----------------------------------------------------------
709+
Specific to FileAppendTransaction, which natively handles chunking.
710+
711+
Verify that:
712+
- freeze_with() correctly generates sequential TransactionIds for each chunk.
713+
- valid_start timestamps for each chunk are spaced out correctly (e.g., incremented by at least 1 nanosecond).
714+
- execute() handles signing and executing each chunk sequentially.
715+
- Validation bounds for max_chunks and chunk_size are strictly enforced.
716+
717+
Flag any change that modifies the multi-chunk loop or skips timestamp incrementation.
718+
719+
----------------------------------------------------------
720+
REVIEW FOCUS 3 — MEMO HANDLING
721+
----------------------------------------------------------
722+
Nuanced memo distinction across the file module:
723+
724+
Verify that:
725+
- The distinction between file_memo (the metadata attribute of the file) and transaction_memo (the note on the transaction itself) is respected.
726+
- FileCreateTransaction handles file_memo as a native string.
727+
- FileUpdateTransaction correctly wraps its memo in Google's StringValue protobuf wrapper to distinguish between an absent unchanged state and an explicit clearing of the memo.
728+
729+
Flag if file_memo string values are mishandled or swapped with transaction_memo.
730+
731+
----------------------------------------------------------
732+
REVIEW FOCUS 4 — TRANSACTION BASE CLASS CONTRACT
733+
----------------------------------------------------------
734+
All file transaction classes MUST inherit from Transaction.
735+
736+
Required implementations:
737+
- _build_proto_body()
738+
- build_transaction_body()
739+
- build_scheduled_body()
740+
- _get_method(channel)
741+
742+
Verify that:
743+
- All setters call self._require_not_frozen() before mutation.
744+
- Chunk payload slicing in build_transaction_body() accurately extracts the current chunk before signing and execution.
745+
746+
----------------------------------------------------------
747+
REVIEW FOCUS 5 — PROTOBUF ALIGNMENT
748+
----------------------------------------------------------
749+
Serialization and deserialization MUST map directly to Hedera protobufs.
750+
751+
Verify that:
752+
- fileCreate, fileUpdate, fileAppend, and fileDelete fields map exactly to their respective protobuf bodies.
753+
- Null-safe conversions are handling optional properties safely.
754+
755+
----------------------------------------------------------
756+
REVIEW FOCUS 6 — TEST EXPECTATIONS
757+
----------------------------------------------------------
758+
Good to check:
759+
- Robust unit and integration tests exist for chunking limits and bounds.
760+
- Examples accurately depict native chunking over manual looping.
761+
762+
----------------------------------------------------------
763+
REVIEW FOCUS 7 — VALIDATION & ERROR BEHAVIOR
764+
----------------------------------------------------------
765+
Required validations must not be deferred to the node.
766+
767+
Verify that:
768+
- Null-checking is aggressively performed (e.g., FileId existence before queries or delete operations).
769+
- Client-side size and bounds validations correctly raise deterministic exceptions prior to network execution.
770+
- File keys are strictly required for creation and update commands.
771+
- Silent no-ops or ignored input states are flagged.
772+
773+
----------------------------------------------------------
774+
EXPLICIT NON-GOALS
775+
----------------------------------------------------------
776+
Do NOT:
777+
- Propose refactors unless they directly improve safety, chunking correctness, or API stability.
778+
- Suggest sweeping style or naming changes unless they are actively misleading.
779+
- Flag logic outside of the file module.
780+
781+
----------------------------------------------------------
782+
FINAL OBJECTIVE
783+
----------------------------------------------------------
784+
Ensure the file service code is:
785+
- Backward-compatible
786+
- Chunking-safe and deterministically executed
787+
- Accurately aligned with protobuf definitions
788+
- Validating inputs consistently
789+
674790
# ============================================================
675791
# GLOBAL REVIEW INSTRUCTIONS (APPLY TO ALL FILES)
676792
# ============================================================
@@ -1147,9 +1263,15 @@ reviews:
11471263
- path: "src/hiero_sdk_python/contract/**/*_query.py"
11481264
instructions: *query_review_instructions
11491265

1266+
- path: "src/hiero_sdk_python/file/**/*_query.py"
1267+
instructions: *query_review_instructions
1268+
11501269
- path: "src/hiero_sdk_python/contract/**"
11511270
instructions: *contract_review_instructions
11521271

1272+
- path: "src/hiero_sdk_python/file/**/*.py"
1273+
instructions: *file_review_instructions
1274+
11531275
chat:
11541276
art: false # Don't draw ASCII art (false)
11551277
auto_reply: false # Don't allow bot to converse (spammy)

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.
1010

1111
- Added CodeRabbit review instructions for the transaction module in `.coderabbit.yaml` (#1696)
1212
- Added CodeRabbit review instructions and path mapping for the schedule module (`src/hiero_sdk_python/schedule/`) in `.coderabbit.yaml` (#1698)
13+
- Added advanced code review prompts for the `src/hiero_sdk_python/file` module in `.coderabbit.yaml` to guide reviewers in verifying proper `FileAppendTransaction` chunking constraints and nuances in memo handling for `FileUpdateTransaction` according to Hiero SDK best practices. (#1697)
1314

1415
### Src
1516
- Fix `TopicInfo.__str__()` to format `expiration_time` in UTC so unit tests pass in non-UTC environments. (#1800)

0 commit comments

Comments
 (0)