feat: Added support for include_children to the TransactionRecordQuery class #1621
Conversation
|
[commit-verification-bot] To achieve verified status, please read: Remember, you require a GPG key and each commit must be signed with: Thank you for contributing! From the Hiero Python SDK Team |
WalkthroughAdds support for including child transaction records in queries by adding an Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Query as TransactionRecordQuery
participant Network as Network/Service
participant Model as TransactionRecord
Client->>Query: set_transaction_id(id)
Client->>Query: set_include_children(true)
Client->>Query: execute(client)
Query->>Query: _make_request()
Query->>Network: getTxRecordByTxID(id, include_child_records=true)
Network-->>Query: response (parent + child records)
Query->>Query: _map_record_list(child_records)
Query->>Model: _from_proto(parent_proto, children=[...])
Model-->>Query: TransactionRecord(with children)
Query-->>Client: TransactionRecord (parent with children)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hiero_sdk_python/transaction/transaction_record.py (1)
174-185: Critical bug:childrenparameter is ignored.The
childrenparameter passed to_from_proto(line 123) is completely overwritten by the local variable at line 174. This means child records passed fromTransactionRecordQuery.execute()are never actually stored in the resultingTransactionRecord.- children: Optional[List["TransactionRecord"]] = None return cls( transaction_id=transaction_id, ... - children=children, + children=children, # Use the parameter, not a new local variableThe fix should remove line 174 entirely so that the
childrenparameter from line 123 is used:🐛 Proposed fix
for pending_airdrop in proto.new_pending_airdrops: new_pending_airdrops.append( PendingAirdropRecord._from_proto(pending_airdrop) ) - children: Optional[List["TransactionRecord"]] = None return cls( transaction_id=transaction_id, transaction_hash=proto.transactionHash,src/hiero_sdk_python/query/transaction_record_query.py (1)
90-92: Avoidprint()statements in production code.Using
print()for error logging is not appropriate for library code. This can cause unexpected output for SDK users and doesn't integrate with their logging configuration.Consider using Python's
loggingmodule or removing the print statement entirely since the exception is re-raised anyway.except Exception as e: - print(f"Exception in _make_request: {e}") raise
| - Formatted client_test.py using Black. | ||
|
|
||
| ### Added | ||
| - Added support for include_children to the TransactionRecordQuery class |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding the issue reference for traceability.
Other entries in this section include issue references (e.g., (#1570), (#1484)). Adding the related issue number would improve traceability.
-- Added support for include_children to the TransactionRecordQuery class
+- Added support for include_children to the TransactionRecordQuery class (`#1512`)📝 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.
| - Added support for include_children to the TransactionRecordQuery class | |
| - Added support for include_children to the TransactionRecordQuery class (`#1512`) |
There was a problem hiding this comment.
@mukundkumarjha please add the issue reference in the CHANGELOG entry to keep it consistent and easy to read.
| @@ -0,0 +1,33 @@ | |||
| import os | |||
There was a problem hiding this comment.
Unused import.
The os module is imported but never used in this example.
-import os📝 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.
| import os |
| def main(): | ||
| # Setup client | ||
| client = Client.for_testnet() | ||
| # ... (add operator credentials) ... | ||
|
|
||
| # 1. Execute a transaction that is likely to have child records | ||
| # (e.g., a complex Smart Contract call) | ||
| print("Executing transaction...") | ||
|
|
||
| # 2. Query the record and explicitly request children | ||
| tx_id = "..." # your transaction id | ||
|
|
||
| query = ( | ||
| TransactionRecordQuery() | ||
| .set_transaction_id(tx_id) | ||
| .set_include_children(True) # The new feature! | ||
| ) | ||
|
|
||
| record = query.execute(client) |
There was a problem hiding this comment.
Example is incomplete and will fail if copy-pasted.
Per coding guidelines, examples must work verbatim for users who copy-paste them. This example has several issues:
- Missing operator credentials:
Client.for_testnet()withoutset_operator()will fail on paid queries. - Invalid transaction ID type: Line 15 uses a string
"...", butset_transaction_id()expects aTransactionIdobject. - Missing imports:
TransactionIdis not imported.
💡 Suggested fix
import os
+from dotenv import load_dotenv
from hiero_sdk_python.client import Client
from hiero_sdk_python.query.transaction_record_query import TransactionRecordQuery
+from hiero_sdk_python.transaction.transaction_id import TransactionId
+from hiero_sdk_python.account.account_id import AccountId
+from hiero_sdk_python.crypto.private_key import PrivateKey
def main():
+ load_dotenv()
+
# Setup client
client = Client.for_testnet()
- # ... (add operator credentials) ...
+ operator_id = AccountId.from_string(os.getenv("OPERATOR_ID"))
+ operator_key = PrivateKey.from_string(os.getenv("OPERATOR_KEY"))
+ client.set_operator(operator_id, operator_key)
# 1. Execute a transaction that is likely to have child records
# (e.g., a complex Smart Contract call)
print("Executing transaction...")
# 2. Query the record and explicitly request children
- tx_id = "..." # your transaction id
+ # Replace with an actual transaction ID from a previous transaction
+ tx_id = TransactionId.from_string("[email protected]")
query = (
TransactionRecordQuery()
.set_transaction_id(tx_id)
.set_include_children(True) # The new feature!
)| def _map_record_list( | ||
| self, | ||
| proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse], | ||
| ) -> List[TransactionRecord]: | ||
| records: List[TransactionRecord] = [] | ||
| for record in proto_records: | ||
| records.append(TransactionRecord._from_proto(record, self.transaction_id)) | ||
|
|
||
| return records |
There was a problem hiding this comment.
Incorrect type hint for proto_records parameter.
The type hint specifies List[transaction_get_record_pb2.TransactionGetRecordResponse], but child_transaction_records from the protobuf response contains TransactionRecord protos, not TransactionGetRecordResponse.
This mismatch is misleading and could cause confusion for future maintainers.
def _map_record_list(
self,
- proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse],
+ proto_records: List[transaction_record_pb2.TransactionRecord],
) -> List[TransactionRecord]:Also need to add the import:
from hiero_sdk_python.hapi.services import (
query_header_pb2,
transaction_get_record_pb2,
query_pb2,
+ transaction_record_pb2,
)|
|
||
| prng_number: Optional[int] = None | ||
| prng_bytes: Optional[bytes] = None | ||
| children: List[TransactionId] = None |
There was a problem hiding this comment.
Type annotation inconsistency.
The field is declared as List[TransactionId], but based on the usage in the example (child.receipt.status) and the mapping logic in TransactionRecordQuery._map_record_list(), this should be List[TransactionRecord].
Additionally, per Ruff UP035, typing.List is deprecated in favor of list.
- children: List[TransactionId] = None
+ children: list["TransactionRecord"] = None📝 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.
| children: List[TransactionId] = None | |
| children: list["TransactionRecord"] = None |
| assert result.transaction_memo == "Test transaction" | ||
|
|
||
|
|
||
| from hiero_sdk_python.hapi.services import query_header_pb2, transaction_get_record_pb2 |
There was a problem hiding this comment.
Remove duplicate imports.
This line redefines query_header_pb2 and transaction_get_record_pb2 that were already imported at lines 10 and 14. This will cause linter errors (Ruff F811) and is unnecessary.
-from hiero_sdk_python.hapi.services import query_header_pb2, transaction_get_record_pb2📝 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.
| from hiero_sdk_python.hapi.services import query_header_pb2, transaction_get_record_pb2 |
🧰 Tools
🪛 Ruff (0.14.14)
97-97: Redefinition of unused query_header_pb2 from line 14: query_header_pb2 redefined here
Remove definition: query_header_pb2
(F811)
97-97: Redefinition of unused transaction_get_record_pb2 from line 10: transaction_get_record_pb2 redefined here
Remove definition: transaction_get_record_pb2
(F811)
| def test_transaction_record_query_with_children_mapping(transaction_id): | ||
|
|
||
| child_proto = transaction_record_pb2.TransactionRecord( | ||
| memo="Child Record" | ||
| ) | ||
| child_proto.transactionID.CopyFrom(transaction_id._to_proto()) | ||
|
|
||
| record_response = transaction_get_record_pb2.TransactionGetRecordResponse() | ||
|
|
||
| record_response.header.nodeTransactionPrecheckCode = ResponseCode.OK | ||
|
|
||
| record_response.transactionRecord.memo = "Parent Record" | ||
| record_response.transactionRecord.transactionID.CopyFrom(transaction_id._to_proto()) | ||
|
|
||
| record_response.child_transaction_records.extend([child_proto, child_proto]) | ||
|
|
||
| query = TransactionRecordQuery(transaction_id) | ||
| children = query._map_record_list(record_response.child_transaction_records) | ||
|
|
||
| assert len(children) == 2 | ||
| assert children[0].transaction_memo == "Child Record" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Test coverage gaps for the new include_children feature.
Per the coding guidelines, unit tests should verify:
- Fluent setters return
self: Missing test forset_include_childrenreturningselffor chaining. - Backward-compatible defaults: Missing test that
include_childrendefaults toFalse. - Constructor behavior: Missing test for
TransactionRecordQuery(transaction_id, include_children=True). - Request construction: Missing test that
_make_request()correctly setsinclude_child_recordson the protobuf.
💚 Suggested additional tests
def test_set_include_children_returns_self(transaction_id):
"""Test that set_include_children returns self for method chaining."""
query = TransactionRecordQuery(transaction_id)
result = query.set_include_children(True)
assert result is query # Fluent interface check
def test_include_children_default_is_false():
"""Test that include_children defaults to False."""
query = TransactionRecordQuery()
assert query.include_children is False
def test_constructor_with_include_children(transaction_id):
"""Test constructor accepts include_children parameter."""
query = TransactionRecordQuery(transaction_id, include_children=True)
assert query.include_children is True
assert query.transaction_id == transaction_id
def test_make_request_includes_children_flag(transaction_id):
"""Test that _make_request sets include_child_records correctly."""
query = TransactionRecordQuery(transaction_id, include_children=True)
request = query._make_request()
assert request.transactionGetRecord.include_child_records is TrueBased on learnings: "Assert fluent setters return self", "Assert backward-compatible defaults are maintained", "Test constructor behavior with valid inputs".
| record_response.child_transaction_records.extend([child_proto, child_proto]) | ||
|
|
||
| query = TransactionRecordQuery(transaction_id) | ||
| children = query._map_record_list(record_response.child_transaction_records) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the transaction_record_query.py file and examine _map_record_list definition
fd "transaction_record_query.py" --type f | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 170
🏁 Script executed:
# Search for _map_record_list definition with context
rg -n "_map_record_list" --type=py -A5 -B2Repository: hiero-ledger/hiero-sdk-python
Length of output: 2355
🏁 Script executed:
# Search for child_transaction_records definition and usage
rg -n "child_transaction_records" --type=py -C3Repository: hiero-ledger/hiero-sdk-python
Length of output: 1857
🏁 Script executed:
# Find protobuf files in the repository
fd -e "proto" --type f | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Look at the test file to understand what child_proto is
sed -n '100,115p' tests/unit/transaction_record_query_test.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 766
🏁 Script executed:
# Search for imports in transaction_record_query.py to understand where types come from
head -30 src/hiero_sdk_python/query/transaction_record_query.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 1157
🏁 Script executed:
# Search for TransactionRecord proto definition or where transaction_get_record_pb2 comes from
rg -n "transaction_get_record_pb2" --type=py | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 1203
🏁 Script executed:
# Find where transaction_record_pb2 and its definitions are
fd -n "transaction_record_pb2" --type fRepository: hiero-ledger/hiero-sdk-python
Length of output: 303
🏁 Script executed:
# Look for protobuf Python files to understand the field definitions
find . -name "*pb2.py" -type f | grep -E "(transaction_record|transaction_get_record)" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Let's check the generated protobuf definitions to see what type child_transaction_records actually is
rg -n "child_transaction_records" --type=py -B5 -A2 | head -50Repository: hiero-ledger/hiero-sdk-python
Length of output: 2025
🏁 Script executed:
# Look at the full implementation of _map_record_list to see what it actually does with proto_records
sed -n '196,210p' src/hiero_sdk_python/query/transaction_record_query.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 591
🏁 Script executed:
# Search for TransactionRecord._from_proto to understand what it expects
rg -n "_from_proto" --type=py -B2 -A5 | grep -A5 "class TransactionRecord"Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Check the imports in transaction_record_query.py to see what transaction_record_pb2 is
rg -n "transaction_record_pb2\|transaction_get_record_pb2" src/hiero_sdk_python/query/transaction_record_query.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Find the TransactionRecord class definition to see what _from_proto expects
fd "transaction_record.py" --type fRepository: hiero-ledger/hiero-sdk-python
Length of output: 127
🏁 Script executed:
# Get the signature of TransactionRecord._from_proto
rg -n "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py -A3Repository: hiero-ledger/hiero-sdk-python
Length of output: 235
🏁 Script executed:
# Check what imports transaction_record_pb2 in the test
sed -n '1,15p' tests/unit/transaction_record_query_test.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 570
Fix type hint for _map_record_list parameter in production code.
The type annotation for _map_record_list in src/hiero_sdk_python/query/transaction_record_query.py (lines 196-199) is incorrect. The parameter proto_records should be List[transaction_record_pb2.TransactionRecord], not List[transaction_get_record_pb2.TransactionGetRecordResponse].
The implementation passes each record directly to TransactionRecord._from_proto, which expects transaction_record_pb2.TransactionRecord objects. The actual field child_transaction_records in TransactionGetRecordResponse contains TransactionRecord protos, as the test correctly demonstrates. Update the type hint to match the actual implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/hiero_sdk_python/account/account_info.py (2)
12-109: Fix staking_info wiring (wrong class + unused variable).
StakingInfois imported from the protobuf module and then used with.from_proto(), which doesn’t exist. Additionally, the computedstaking_infois never assigned toaccount_info, so it’s always lost. This will either raise at runtime or silently drop staking data.🐛 Proposed fix
-from hiero_sdk_python.hapi.services.basic_types_pb2 import StakingInfo +from hiero_sdk_python.staking_info import StakingInfo ... - account_info: "AccountInfo" = cls( + account_info: "AccountInfo" = cls( account_id=AccountId._from_proto(proto.accountID) if proto.accountID else None, contract_account_id=proto.contractAccountID, is_deleted=proto.deleted, proxy_received=Hbar.from_tinybars(proto.proxyReceived), key=PublicKey._from_proto(proto.key) if proto.key else None, balance=Hbar.from_tinybars(proto.balance), receiver_signature_required=proto.receiverSigRequired, expiration_time=( Timestamp._from_protobuf(proto.expirationTime) if proto.expirationTime else None ), auto_renew_period=( Duration._from_proto(proto.autoRenewPeriod) if proto.autoRenewPeriod else None ), token_relationships=[ TokenRelationship._from_proto(relationship) for relationship in proto.tokenRelationships ], account_memo=proto.memo, owned_nfts=proto.ownedNfts, max_automatic_token_associations=proto.max_automatic_token_associations, + staking_info=( + StakingInfo._from_proto(proto.staking_info) + if proto.HasField("staking_info") + else None + ), ) - - staking_info=( - StakingInfo.from_proto(proto.staking_info) - if proto.HasField("staking_info") - else None - )And in
_to_proto:- staking_info=( - self.staking_info.to_proto() - if self.staking_info is not None - else None - ), + staking_info=( + self.staking_info._to_proto() + if self.staking_info is not None + else None + ),
161-199: Fix staking_info labels in str/repr.
Current output prints the wholestaking_infoobject for “Staked Account ID”, “Staked Node ID”, and “Decline Staking Reward”. This is misleading.💡 Suggested adjustment
- (self.staking_info, "Staked Account ID"), - (self.staking_info, "Staked Node ID"), + (self.staking_info.staked_account_id if self.staking_info else None, "Staked Account ID"), + (self.staking_info.staked_node_id if self.staking_info else None, "Staked Node ID"), ... - if self.staking_info is not None: - lines.append(f"Decline Staking Reward: {self.staking_info}") + if self.staking_info is not None: + lines.append( + f"Decline Staking Reward: {self.staking_info.decline_reward}" + ) ... - f"staked_node_id={self.staking_info!r}, " - f"staked_account_id={self.staking_info!r}" + f"staking_info={self.staking_info!r}"src/hiero_sdk_python/transaction/transaction_record.py (1)
119-186: Children are discarded in _from_proto.
The method acceptschildrenbut immediately overwrites it withNone, so include‑children never surfaces in returned records.🐛 Proposed fix
- def _from_proto( + def _from_proto( cls, proto: transaction_record_pb2.TransactionRecord, transaction_id: Optional[TransactionId] = None, - children=None, + children: Optional[list["TransactionRecord"]] = None, ) -> "TransactionRecord": ... - children: Optional[List["TransactionRecord"]] = None return cls( transaction_id=transaction_id, transaction_hash=proto.transactionHash, transaction_memo=proto.memo, transaction_fee=proto.transactionFee, receipt=TransactionReceipt._from_proto(proto.receipt, transaction_id), token_transfers=token_transfers, nft_transfers=nft_transfers, transfers=transfers, new_pending_airdrops=new_pending_airdrops, - children=children, + children=children, call_result=( ContractFunctionResult._from_proto(proto.contractCallResult) if proto.HasField("contractCallResult") else None ), prng_number=proto.prng_number, prng_bytes=proto.prng_bytes, )src/hiero_sdk_python/query/transaction_record_query.py (1)
90-92: RemoveThe
Suggested fix
except Exception as e: - print(f"Exception in _make_request: {e}") raise
CHANGELOG.md
Outdated
| <<<<<<< HEAD | ||
| - Refactored AccountInfo class to use the staking_info | ||
| ======= | ||
| - chore: format tests/unit/mock_server.py with black (#1542) | ||
| >>>>>>> upstream |
There was a problem hiding this comment.
Resolve the merge conflict block before release.
The changelog contains conflict markers, which will break rendering and automation. Please remove the markers and keep the correct entries.
🔧 Suggested resolution
-<<<<<<< HEAD
-- Refactored AccountInfo class to use the staking_info
-=======
-- chore: format tests/unit/mock_server.py with black (`#1542`)
->>>>>>> upstream
+- Refactored AccountInfo class to use the staking_info
+- chore: format tests/unit/mock_server.py with black (`#1542`)📝 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.
| <<<<<<< HEAD | |
| - Refactored AccountInfo class to use the staking_info | |
| ======= | |
| - chore: format tests/unit/mock_server.py with black (#1542) | |
| >>>>>>> upstream | |
| - Refactored AccountInfo class to use the staking_info | |
| - chore: format tests/unit/mock_server.py with black (`#1542`) |
Signed-off-by: Jessica G <[email protected]>
Signed-off-by: mukundkumarjha <[email protected]>
Signed-off-by: mukundkumarjha <[email protected]>
Signed-off-by: mukundkumarjha <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hiero_sdk_python/transaction/transaction_record.py (1)
174-186: Preserve provided child records (currently overwritten toNone).Line 174 resets
childrentoNone, discarding the list passed fromTransactionRecordQuery.execute(). This breaks the newinclude_childrenfeature because the parent record will never retain its child records.🐛 Proposed fix
- children: Optional[List["TransactionRecord"]] = None + if children is None: + children = [] return cls( transaction_id=transaction_id, transaction_hash=proto.transactionHash, transaction_memo=proto.memo, transaction_fee=proto.transactionFee, receipt=TransactionReceipt._from_proto(proto.receipt, transaction_id), token_transfers=token_transfers, nft_transfers=nft_transfers, transfers=transfers, new_pending_airdrops=new_pending_airdrops, - children=children, + children=children, call_result=( ContractFunctionResult._from_proto(proto.contractCallResult) if proto.HasField("contractCallResult") else None ), prng_number=proto.prng_number, prng_bytes=proto.prng_bytes, )
| return self | ||
|
|
||
| def set_include_children(self, include_children): | ||
| self.include_children = include_children |
There was a problem hiding this comment.
Minor suggestion consider adding a docstring
| self.transaction_id : Optional[TransactionId] = transaction_id | ||
|
|
||
| self.transaction_id: Optional[TransactionId] = transaction_id | ||
| self.include_children = include_children |
There was a problem hiding this comment.
missing type hint
| self.include_children = include_children | |
| self.include_children: Optional[bool] = include_children |
| def __init__( | ||
| self, | ||
| transaction_id: Optional[TransactionId] = None, | ||
| include_children: bool = False, |
There was a problem hiding this comment.
this should be include_children: Optional[bool] = False
|
|
||
| def _map_record_list( | ||
| self, | ||
| proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse], |
There was a problem hiding this comment.
must be
| proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse], | |
| proto_records: List[transaction_record_pb2.TransactionRecord], |
|
|
||
| prng_number: Optional[int] = None | ||
| prng_bytes: Optional[bytes] = None | ||
| children: List[TransactionId] = None |
There was a problem hiding this comment.
must be
| children: List[TransactionId] = None | |
| children: list["TransactionRecord"] = None |
| cls, | ||
| proto: transaction_record_pb2.TransactionRecord, | ||
| transaction_id: Optional[TransactionId] = None, | ||
| children=None, |
There was a problem hiding this comment.
add type hinting children: Optional[List["TransactionRecord"]]=[]
| PendingAirdropRecord._from_proto(pending_airdrop) | ||
| ) | ||
|
|
||
| children: Optional[List["TransactionRecord"]] = None |
There was a problem hiding this comment.
this line can be romove
manishdait
left a comment
manishdait
left a comment
There was a problem hiding this comment.
@mukundkumarjha, Thanks for PR added some required changes
manishdait
left a comment
manishdait
left a comment
There was a problem hiding this comment.
@mukundkumarjha, Thanks for PR added some required changes
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@mukundkumarjha Thank you for your contribution!
I concur with @manishdait with the suggested changes.
If you need assistance please reach out.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
|
|
||
| def main(): | ||
| # Setup client | ||
| client = Client.for_testnet() |
There was a problem hiding this comment.
our for testnet mehod does not set the credentials
@classmethod
def for_testnet(cls) -> "Client":
"""
Create a Client configured for Hedera Testnet.
Note: Operator must be set manually using set_operator().
Returns:
Client: A Client instance configured for testnet.
"""
return cls(Network("testnet"))
I would recommend you use
@classmethod
def from_env(cls, network: Optional[NetworkName] = None) -> "Client":
"""
Initialize client from environment variables.
Automatically loads .env file if present.
Args:
network (str, optional): Override the network ("testnet", "mainnet", "previewnet").
If not provided, checks 'NETWORK' env var.
Defaults to 'testnet' if neither is set.
Raises:
ValueError: If OPERATOR_ID or OPERATOR_KEY environment variables are not set.
Example:
# Defaults to testnet if no env vars set
client = Client.from_env()
"""
| # ... (add operator credentials) ... | ||
|
|
||
| # 1. Execute a transaction that is likely to have child records | ||
| # (e.g., a complex Smart Contract call) |
There was a problem hiding this comment.
can we add the example, so we can run it?
There was a problem hiding this comment.
I am not sure how to do that, can you please co author
Signed-off-by: Mukund Jha <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hiero_sdk_python/transaction/transaction_record.py (1)
123-186:⚠️ Potential issue | 🔴 Critical
childrenis discarded, so include_children always yields None.
Line 174 overwrites thechildrenparameter withNone, soTransactionRecordQuery.execute()can never populate children and the example will crash when it callslen(record.children).🔧 Proposed fix
- children: Optional[List["TransactionRecord"]] = None + if children is None: + children = [] return cls( @@ - children=children, + children=children,
| record = query.execute(client) | ||
|
|
||
| # 3. Demonstrate accessing the children | ||
| print(f"Parent Transaction ID: {record.transaction_id}") | ||
| print(f"Number of child records found: {len(record.children)}") | ||
|
|
||
| for i, child in enumerate(record.children): | ||
| print(f"Child {i+1} Status: {child.receipt.status}") | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the file in question
fd "transaction_record_with_children.py" --type fRepository: hiero-ledger/hiero-sdk-python
Length of output: 129
🏁 Script executed:
# Check if the file exists and read its content
if [ -f "examples/transaction/transaction_record_with_children.py" ]; then
wc -l examples/transaction/transaction_record_with_children.py
echo "---"
cat -n examples/transaction/transaction_record_with_children.py
else
echo "File not found"
fiRepository: hiero-ledger/hiero-sdk-python
Length of output: 1383
🏁 Script executed:
# Verify ResponseCode exists in hiero_sdk_python
find . -type f -name "*.py" | xargs grep -l "class ResponseCode" | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 112
🏁 Script executed:
# Check how ResponseCode is imported in other example files
find examples -type f -name "*.py" -exec grep -l "ResponseCode" {} \; | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 3862
🏁 Script executed:
# Look for patterns of receipt.status usage in examples
rg "receipt\.status" examples --type py -B 2 -A 2 | head -40Repository: hiero-ledger/hiero-sdk-python
Length of output: 3540
🏁 Script executed:
# Check how ResponseCode is imported in those example files
head -20 examples/tokens/token_fee_schedule_update_transaction_fungible.py | grep -E "^(import|from)" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 498
🏁 Script executed:
# Check if there's a general pattern for error handling in examples
head -30 examples/tokens/token_create_transaction_token_fee_schedule_key.py | grep -E "^(import|from)" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 454
🏁 Script executed:
# Verify the ResponseCode import path can be used as suggested
head -5 src/hiero_sdk_python/response_code.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 154
🏁 Script executed:
# Check other transaction_record examples for patterns
find examples -name "*transaction_record*" -type fRepository: hiero-ledger/hiero-sdk-python
Length of output: 172
🏁 Script executed:
# Look at how examples handle query results without receipt checks (queries vs transactions)
rg "\.execute\(client\)" examples/query --type py -A 3 | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 988
🏁 Script executed:
# Check the other transaction_record query example
cat -n examples/query/transaction_record_query.py | head -40Repository: hiero-ledger/hiero-sdk-python
Length of output: 1536
🏁 Script executed:
# Verify ResponseCode is used consistently in query examples
rg "ResponseCode" examples/query --type py -B 1 -A 1 | head -30Repository: hiero-ledger/hiero-sdk-python
Length of output: 2088
Validate child receipt status using ResponseCode enum.
Line 30 prints the raw numeric status without converting to ResponseCode, making it unclear to users whether children succeeded or failed. This contradicts the pattern used throughout examples where all receipt statuses are validated against ResponseCode.SUCCESS.
✅ Suggested improvement
from hiero_sdk_python.query.transaction_record_query import TransactionRecordQuery
+from hiero_sdk_python.response_code import ResponseCode
@@
for i, child in enumerate(record.children):
- print(f"Child {i+1} Status: {child.receipt.status}")
+ status = ResponseCode(child.receipt.status)
+ print(f"Child {i+1} Status: {status.name}")
+ if status != ResponseCode.SUCCESS:
+ print(f"Warning: Child {i+1} failed with status {status.name}")| def _map_record_list( | ||
| self, | ||
| proto_records: List[transaction_get_record_pb2.TransactionGetRecordResponse], | ||
| ) -> List[TransactionRecord]: | ||
| records: List[TransactionRecord] = [] | ||
| for record in proto_records: | ||
| records.append(TransactionRecord._from_proto(record, self.transaction_id)) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the file to understand the context
cat -n src/hiero_sdk_python/query/transaction_record_query.py | head -220 | tail -50Repository: hiero-ledger/hiero-sdk-python
Length of output: 2151
🏁 Script executed:
# Find the TransactionRecord._from_proto method to understand its signature
fd -e py -path "*/transaction_record*" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 241
🏁 Script executed:
# Search for TransactionRecord class and _from_proto method
rg -A 10 "def _from_proto" --type py -B 2 | grep -A 10 "TransactionRecord"Repository: hiero-ledger/hiero-sdk-python
Length of output: 1387
🏁 Script executed:
# Look at the protobuf definition for TransactionGetRecordResponse
fd -e proto | xargs rg -l "TransactionGetRecordResponse" | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 171
🏁 Script executed:
# Get the full TransactionRecord._from_proto method implementation
rg -B 5 -A 30 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | head -60Repository: hiero-ledger/hiero-sdk-python
Length of output: 1636
🏁 Script executed:
# Check what fields are in the protobuf TransactionGetRecordResponse
fd -name "*.proto" | xargs rg "TransactionGetRecordResponse" -A 10 2>/dev/null | head -40Repository: hiero-ledger/hiero-sdk-python
Length of output: 3834
🏁 Script executed:
# Look for test file to understand intended behavior
cat -n tests/unit/transaction_record_query_test.py | head -100Repository: hiero-ledger/hiero-sdk-python
Length of output: 4438
🏁 Script executed:
# Search for how transaction_id is used in TransactionRecord class
rg "transaction_id" src/hiero_sdk_python/transaction/transaction_record.py -B 2 -A 2 | head -50Repository: hiero-ledger/hiero-sdk-python
Length of output: 2216
🏁 Script executed:
# Find proto files with transaction record definitions
find . -name "*.proto" -type f | xargs grep -l "TransactionGetRecordResponse\|child_transaction_records" 2>/dev/nullRepository: hiero-ledger/hiero-sdk-python
Length of output: 57
🏁 Script executed:
# Search for TransactionGetRecordResponse definition
find . -name "*.proto" -type f -exec grep -l "TransactionGetRecordResponse" {} \; | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Look for the actual proto definitions in generated Python code
grep -r "child_transaction_records" --include="*.py" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 461
🏁 Script executed:
# Check what type child_transaction_records actually is
rg "child_transaction_records" -B 3 -A 3 --type py | head -50Repository: hiero-ledger/hiero-sdk-python
Length of output: 1789
🏁 Script executed:
# Look at the full test to understand the structure
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1424
🏁 Script executed:
# Read more of the test file to see if there's verification of child transaction IDs
cat tests/unit/transaction_record_query_test.py | sed -n '98,150p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 2312
🏁 Script executed:
# Check the protobuf message structure - look at generated Python code
find . -name "*transaction_record_pb2.py" -o -name "*transaction_get_record_pb2.py" | head -3Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Look at the generated protobuf to understand child_transaction_records structure
rg "class TransactionRecord|child_transaction_records" --type py -A 2 -B 1 | grep -A 5 "child_transaction_records"Repository: hiero-ledger/hiero-sdk-python
Length of output: 1097
🏁 Script executed:
# Check how child records are used elsewhere in the codebase
rg "child_transaction_records\|children" src/hiero_sdk_python/query/transaction_record_query.py -B 2 -A 2Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Let's look at the full _get_query_response method to see how _map_record_list is called
cat -n src/hiero_sdk_python/query/transaction_record_query.py | sed -n '145,210p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 2799
🏁 Script executed:
# Check what the actual type of child_transaction_records is in the protobuf
python3 << 'EOF'
import sys
sys.path.insert(0, '/root')
from hiero_sdk_python.hapi.services import transaction_get_record_pb2
# Inspect the TransactionGetRecordResponse structure
msg = transaction_get_record_pb2.TransactionGetRecordResponse()
# Check field descriptors
for field in msg.DESCRIPTOR.fields:
if 'child' in field.name.lower():
print(f"Field: {field.name}")
print(f" Type: {field.message_type.name if field.message_type else 'scalar'}")
print(f" Label: {field.label}")
EOFRepository: hiero-ledger/hiero-sdk-python
Length of output: 201
🏁 Script executed:
# Now let's verify if child records actually have their own transactionID
python3 << 'EOF'
import sys
sys.path.insert(0, '/root')
from hiero_sdk_python.hapi.services import transaction_record_pb2
# Check if TransactionRecord has transactionID field
msg = transaction_record_pb2.TransactionRecord()
print("TransactionRecord fields:")
for field in msg.DESCRIPTOR.fields:
if 'transaction' in field.name.lower():
print(f" {field.name}: {field.message_type.name if field.message_type else field.type}")
EOFRepository: hiero-ledger/hiero-sdk-python
Length of output: 201
🏁 Script executed:
# Check what happens when TransactionRecord._from_proto receives a transaction_id parameter
rg -A 50 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | head -80Repository: hiero-ledger/hiero-sdk-python
Length of output: 2543
🏁 Script executed:
# Look at proto files to understand the structure
find . -path "*hapi*" -name "*.proto" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Search for child_transaction_records definition in proto
find . -path "*hapi*" -name "*.proto" -exec grep -l "child_transaction_records" {} \;Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Check generated protobuf to see field structure
grep -r "child_transaction_records" src/hiero_sdk_python/hapi --include="*.py" -B 2 -A 2 | head -30Repository: hiero-ledger/hiero-sdk-python
Length of output: 131
🏁 Script executed:
# Most importantly, look at what the test expects - check if child records preserve their own transaction IDs
cat tests/unit/transaction_record_query_test.py | sed -n '98,120p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 996
🏁 Script executed:
# Check the actual implementation of TransactionRecord._from_proto to see how it handles transaction_id parameter
rg -A 100 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | grep -A 10 "transaction_id" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 954
🏁 Script executed:
# Let's check what the actual transaction_id of child records would be after mapping
# by examining if there's any assertion about transaction_id in the test
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1424
🏁 Script executed:
# Check the full _from_proto implementation to understand how it handles the transaction_id parameter
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -A 200 "def _from_proto" | head -100Repository: hiero-ledger/hiero-sdk-python
Length of output: 4644
🏁 Script executed:
# Look for the return statement in _from_proto to see how transaction_id is assigned
rg -A 150 "def _from_proto" src/hiero_sdk_python/transaction/transaction_record.py | grep -B 5 "return cls"Repository: hiero-ledger/hiero-sdk-python
Length of output: 275
🏁 Script executed:
# Check if there's an assertion about transaction_id in the child mapping test that we might have missed
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1275
🏁 Script executed:
# Look at what proto type child_transaction_records actually holds - check the protobuf structure
grep -r "child_transaction_records" . --include="*.py" | grep -v ".pyc" | grep -v "test" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 213
🏁 Script executed:
# Search for TransactionGetRecordResponse in generated protobuf to see its actual fields
find . -name "*.py" -exec grep -l "class TransactionGetRecordResponse" {} \;Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Let's check the actual behavior by looking at what happens when _from_proto is called without explicit transaction_id
rg "if proto.HasField\(\"transactionID\"\)|proto.transactionID" src/hiero_sdk_python/transaction/transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 153
🏁 Script executed:
# Verify if TransactionRecord._from_proto actually extracts transaction_id from proto when not provided
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -B 5 "transaction_id=transaction_id" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 277
🏁 Script executed:
# Let's check if the test actually verifies the transaction_id of child records
cat tests/unit/transaction_record_query_test.py | sed -n '98,130p' | grep -i "transaction_id\|assert"Repository: hiero-ledger/hiero-sdk-python
Length of output: 440
🏁 Script executed:
# Check if there are any assertions about transaction_id in child records
rg "assert.*transaction_id" tests/unit/transaction_record_query_test.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 267
🏁 Script executed:
# Look at the entire test to see all assertions
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1275
🏁 Script executed:
# Check if proto.HasField works on the child records and what fields are available
rg "HasField" src/hiero_sdk_python/transaction/transaction_record.py | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 128
🏁 Script executed:
# Let's check the actual type annotation issue - is it TransactionRecord or TransactionGetRecordResponse?
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '196,210p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 591
🏁 Script executed:
# Critical check: Does TransactionRecord._from_proto actually extract transaction_id from proto when None?
cat src/hiero_sdk_python/transaction/transaction_record.py | sed -n '150,250p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 4742
🏁 Script executed:
# Let me verify the exact test setup - what are the child_proto transaction IDs before mapping?
cat tests/unit/transaction_record_query_test.py | sed -n '98,125p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1275
🏁 Script executed:
# Most importantly, check what the test asserts about child transaction IDs after mapping
rg "children\[0\]\.transaction_id|assert.*transaction" tests/unit/transaction_record_query_test.py | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 527
🏁 Script executed:
# Check if there are other tests that validate child record transaction IDs
rg -C 5 "child.*transaction_id\|transaction_id.*child" tests/Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Final verification: check if proto records actually have their own transactionID field
cat src/hiero_sdk_python/transaction/transaction_record.py | grep -A 5 "def _from_proto" | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 272
🏁 Script executed:
# Look for any extraction of transactionID from proto in the _from_proto method
rg "proto\.transactionID|HasField.*transactionID" src/hiero_sdk_python/transaction/transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 153
🏁 Script executed:
# Check what the test file setup shows - each child has transactionID set on proto
cat tests/unit/transaction_record_query_test.py | sed -n '98,110p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 621
🏁 Script executed:
# Confirm: the type annotation says TransactionGetRecordResponse but should be TransactionRecord
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '196,205p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 427
🏁 Script executed:
# Let me verify the type annotation issue - what is actually in child_transaction_records?
cat tests/unit/transaction_record_query_test.py | sed -n '100,113p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 634
🏁 Script executed:
# Search for how the Hedera SDK JavaScript or other implementations handle child records
rg -i "child.*transaction.*record\|_map_record" --type py -B 2 -A 5 | head -50Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Check the complete context around where _map_record_list is called
cat src/hiero_sdk_python/query/transaction_record_query.py | sed -n '130,160p'Repository: hiero-ledger/hiero-sdk-python
Length of output: 1105
🏁 Script executed:
# Critical: verify the exact type by looking at the proto definitions
find . -name "*transaction_get_record_pb2.py" | xargs grep -A 10 "class TransactionGetRecordResponse" | head -30Repository: hiero-ledger/hiero-sdk-python
Length of output: 55
🏁 Script executed:
# Also check if there's a way to extract transactionID from the child record proto
grep -r "transactionID" src/hiero_sdk_python/transaction/transaction_record.py | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 153
🏁 Script executed:
# Let's create a simple test to understand the behavior
python3 << 'EOF'
# Check if the docstring promise in _from_proto is actually implemented
import re
with open('src/hiero_sdk_python/transaction/transaction_record.py', 'r') as f:
content = f.read()
# Find the _from_proto method
match = re.search(r'def _from_proto\((.*?)\) -> "TransactionRecord":(.*?)return cls\(', content, re.DOTALL)
if match:
method_body = match.group(2)
# Check if it extracts transaction_id from proto
if 'proto.transactionID' in method_body or 'HasField("transactionID")' in method_body:
print("FOUND: extraction of transactionID from proto")
else:
print("NOT FOUND: extraction of transactionID from proto")
print("The _from_proto does NOT extract transaction_id from proto when parameter is None")
EOFRepository: hiero-ledger/hiero-sdk-python
Length of output: 204
Type annotation incorrect; child records lose their own transaction IDs.
Line 198's type annotation should reflect that proto_records contains TransactionRecord protobuf messages, not TransactionGetRecordResponse. More importantly, line 202 passes self.transaction_id to every child record, which overwrites each child's own transactionID field. Extract each child's ID from the protobuf instead, falling back to the parent ID only if the child lacks one.
🔧 Proposed fix
records: List[TransactionRecord] = []
for record in proto_records:
- records.append(TransactionRecord._from_proto(record, self.transaction_id))
+ record_tx_id = (
+ TransactionId._from_proto(record.transactionID)
+ if record.HasField("transactionID")
+ else self.transaction_id
+ )
+ records.append(TransactionRecord._from_proto(record, record_tx_id))| nft_transfers: defaultdict[TokenId, list[TokenNftTransfer]] = field( | ||
| default_factory=lambda: defaultdict(list[TokenNftTransfer]) | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/hiero_sdk_python/transaction && wc -l transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 98
🏁 Script executed:
cd src/hiero_sdk_python/transaction && sed -n '70,80p' transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 524
🏁 Script executed:
cd src/hiero_sdk_python/transaction && sed -n '172,180p' transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 437
🏁 Script executed:
# Check imports and type definitions
cd src/hiero_sdk_python/transaction && head -50 transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 2821
🏁 Script executed:
# Verify if list[TokenNftTransfer] is callable in Python
python3 << 'EOF'
from typing import List
class TokenNftTransfer:
pass
# Test if list[TokenNftTransfer] is callable
type_alias = list[TokenNftTransfer]
print(f"Type of list[TokenNftTransfer]: {type(type_alias)}")
print(f"Is it callable? {callable(type_alias)}")
# Try to call it
try:
result = type_alias()
print(f"Success: {result}")
except TypeError as e:
print(f"Error when calling: {e}")
# Compare with list
print(f"\nIs list callable? {callable(list)}")
print(f"Calling list(): {list()}")
EOFRepository: hiero-ledger/hiero-sdk-python
Length of output: 209
🏁 Script executed:
# Check more context around the nft_transfers field and how it's used
cd src/hiero_sdk_python/transaction && sed -n '60,90p' transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 1417
🏁 Script executed:
# Search for usages of nft_transfers to understand intended behavior
cd src/hiero_sdk_python/transaction && grep -n "nft_transfers" transaction_record.py | head -20Repository: hiero-ledger/hiero-sdk-python
Length of output: 606
🏁 Script executed:
# Check Python version requirements
fd -t f pyproject.toml setup.py setup.cfg | head -5Repository: hiero-ledger/hiero-sdk-python
Length of output: 224
🏁 Script executed:
# Read project config to see Python version requirement
cat pyproject.toml | grep -A 5 "python"Repository: hiero-ledger/hiero-sdk-python
Length of output: 839
🏁 Script executed:
# Check if there are any tests showing expected behavior
grep -r "nft_transfers" --include="*.py" | grep -E "test|spec" | head -10Repository: hiero-ledger/hiero-sdk-python
Length of output: 1132
🏁 Script executed:
# Check actual test to understand expected behavior
cat tests/unit/token_airdrop_transaction_test.py | grep -A 10 "nft_transfer = airdrop_tx.nft_transfers"Repository: hiero-ledger/hiero-sdk-python
Length of output: 910
🏁 Script executed:
# Check the _from_proto method implementation to see what type it expects
cd src/hiero_sdk_python/transaction && sed -n '150,165p' transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 925
🏁 Script executed:
# Verify runtime behavior by testing the actual field initialization
python3 << 'EOF'
from collections import defaultdict
from typing import List
class TokenNftTransfer:
def __repr__(self):
return "TokenNftTransfer()"
# Simulate the current code
nft_transfers = defaultdict(lambda: defaultdict(list[TokenNftTransfer]))
# Access a key that doesn't exist
token_id = "token_123"
result = nft_transfers[token_id]
print(f"Type returned: {type(result)}")
print(f"Is it a defaultdict? {isinstance(result, defaultdict)}")
print(f"Can iterate? {hasattr(result, '__iter__')}")
# Try to append like it's a list
try:
result.append(TokenNftTransfer())
print("ERROR: append succeeded on defaultdict")
except AttributeError as e:
print(f"AttributeError (expected): {e}")
# Try to assign like current code does at line 161
test_list = [TokenNftTransfer(), TokenNftTransfer()]
nft_transfers[token_id] = test_list
print(f"After direct assignment, type: {type(nft_transfers[token_id])}")
print(f"Can iterate? {list(nft_transfers[token_id])}")
EOFRepository: hiero-ledger/hiero-sdk-python
Length of output: 351
🏁 Script executed:
# Verify what _from_proto returns
cd src/hiero_sdk_python/tokens && grep -A 20 "def _from_proto" token_nft_transfer.py | head -25Repository: hiero-ledger/hiero-sdk-python
Length of output: 829
🏁 Script executed:
# Double-check the field initialization and usage pattern
cd src/hiero_sdk_python/transaction && sed -n '239,245p' transaction_record.pyRepository: hiero-ledger/hiero-sdk-python
Length of output: 468
Fix type mismatch in nft_transfers factory: should return list, not defaultdict.
The field is declared as defaultdict[TokenId, list[TokenNftTransfer]], so values must be lists. However, the factory creates a defaultdict(list[TokenNftTransfer]), which returns a nested defaultdict instead of a list. While _from_proto directly assigns lists (bypassing the factory), accessing an uninitialized key would return a defaultdict, causing a runtime type mismatch with code expecting a list (e.g., indexing or iteration).
🔧 Proposed fix
nft_transfers: defaultdict[TokenId, list[TokenNftTransfer]] = field(
- default_factory=lambda: defaultdict(list[TokenNftTransfer])
+ default_factory=list
)
prajeeta15
left a comment
prajeeta15
left a comment
There was a problem hiding this comment.
@mukundkumarjha please address all the coderabbitai issues before pushing code.
| - Formatted client_test.py using Black. | ||
|
|
||
| ### Added | ||
| - Added support for include_children to the TransactionRecordQuery class |
There was a problem hiding this comment.
@mukundkumarjha please add the issue reference in the CHANGELOG entry to keep it consistent and easy to read.
|
@mukundkumarjha |
6 participants
Description:
Related issue(s):
Fixes #1512
Notes for reviewer:
Checklist