Remove chunks parameter from TopicMessage initialization#66
Closed
Remove chunks parameter from TopicMessage initialization#66
Conversation
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
This script posts a review comment on release PRs for CodeRabbit, ensuring it only runs for maintainer-authored PRs and avoids duplicate comments. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Refactor error handling and improve prompt loading in release PR script. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
|
Hi @MonaaEid, this is **LinkBot** 👋
Linking pull requests to issues helps us significantly with reviewing pull requests and keeping the repository healthy. 🚨 This pull request does not have an issue linked. Please link an issue using the following format: 📖 Guide: If no issue exists yet, please create one: Thanks! |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. WalkthroughReplaced TransactionId-typed transaction_id with a string representation (Optional[str]) across TopicMessage and TopicMessageChunk; transaction_id is now constructed as a formatted string from proto fields in both Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/hiero_sdk_python/consensus/topic_message.py (3)
80-89:⚠️ Potential issue | 🔴 Critical
of_singlepasses too many positional arguments —TypeErroron every call.The new
__init__signature is(self, consensus_timestamp, message_data, transaction_id=None), but theof_singleconstructor call at lines 80–89 was not updated to match. It still passes[chunk]as the 3rd positional argument (incorrectly bound totransaction_id) and the realtransaction_idas a 4th argument, which has no corresponding parameter and raises aTypeError.🐛 Proposed fix
return cls( consensus_timestamp, { "contents": contents, "running_hash": running_hash, "sequence_number": sequence_number, }, - [chunk], - transaction_id + transaction_id, )
177-191:⚠️ Potential issue | 🔴 Critical
self.chunksno longer exists —AttributeErroron every__str__call.Line 188 still references
self.chunks, which was removed from__init__. Any call tostr(topic_message)or implicit string conversion will raiseAttributeError.🐛 Proposed fix — remove or replace the stale `chunk_count` field
return ( f"TopicMessage(" f"consensus_timestamp={self.consensus_timestamp}, " f"sequence_number={self.sequence_number}, " f"contents='{contents_str[:40]}{'...' if len(contents_str) > 40 else ''}', " - f"chunk_count={len(self.chunks)}, " f"transaction_id={self.transaction_id}" f")" )
46-57:⚠️ Potential issue | 🟡 MinorStale docstring still documents the removed
chunksparameter.The docstring on lines 55–56 still lists
chunks (List[TopicMessageChunk])as an argument even though it was removed from the signature.📝 Proposed fix
Args: consensus_timestamp (datetime): The final consensus timestamp. message_data (Dict[str, Union[bytes, int]]): Dict with required fields: { "contents": bytes, "running_hash": bytes, "sequence_number": int } - chunks (List[TopicMessageChunk]): All individual chunks that form this message. transaction_id (Optional[Transaction]): The transaction ID if available.
Remove message_data parameter from the constructor. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/hiero_sdk_python/consensus/topic_message.py (3)
79-88:⚠️ Potential issue | 🔴 Critical
of_singlestill passes stale[chunk]argument — every call raisesTypeError.After removing
chunksfrom__init__, the signature is(self, consensus_timestamp, message_data, transaction_id=None). Thecls(...)call at line 79 passes four positional arguments —consensus_timestamp, the dict,[chunk], andtransaction_id— so[chunk]is incorrectly mapped totransaction_idand the actualtransaction_idbecomes a surplus positional argument, raisingTypeErrorat runtime on every invocation ofof_single(and_from_protofor single-chunk paths).🐛 Proposed fix — remove the stale `[chunk]` argument
return cls( consensus_timestamp, { "contents": contents, "running_hash": running_hash, "sequence_number": sequence_number, }, - [chunk], transaction_id )
183-190:⚠️ Potential issue | 🔴 Critical
__str__references the removedself.chunksattribute — raisesAttributeErroron every call.Line 187 reads
len(self.chunks), butself.chunkswas deleted from__init__. Any code that logs, prints, or formats aTopicMessageinstance will crash withAttributeError: 'TopicMessage' object has no attribute 'chunks'.🐛 Proposed fix — remove the stale `chunk_count` field
return ( f"TopicMessage(" f"consensus_timestamp={self.consensus_timestamp}, " f"sequence_number={self.sequence_number}, " f"contents='{contents_str[:40]}{'...' if len(contents_str) > 40 else ''}', " - f"chunk_count={len(self.chunks)}, " f"transaction_id={self.transaction_id}" f")" )
46-55:⚠️ Potential issue | 🔴 CriticalCritical refactoring issues in
TopicMessage.__init__— constructor is broken and will fail at runtime.The
__init__signature accepts onlyconsensus_timestampandtransaction_id, but the body references undefined parametermessage_dataat lines ~58–60 (message_data["contents"],message_data["running_hash"],message_data["sequence_number"]), causingNameError. The docstring still documents bothmessage_dataand removedchunksparameters. Theof_singlemethod (line ~85–88) passes 3 positional arguments to a constructor accepting only 2, causingTypeError.Either restore
message_dataas a parameter or refactor the body to not use it, updateof_singleto match the new signature, and remove the stale docstring entries.
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/hiero_sdk_python/consensus/topic_message.py (3)
79-88:⚠️ Potential issue | 🔴 Critical
of_singlepasses a stale[chunk]positional argument that causesTypeErrorat runtime.The new
__init__signature is(consensus_timestamp, message_data, transaction_id=None)— three parameters.of_singlestill passes four positional arguments:consensus_timestamp, the dict,[chunk], andtransaction_id. This means[chunk](a list) is bound totransaction_id, and the actualtransaction_idvalue becomes an unexpected fifth argument, producing:TypeError: TopicMessage.__init__() takes from 3 to 4 positional arguments but 5 were givenEvery call path through
of_singleand_from_protofor single responses will crash.🐛 Proposed fix
return cls( consensus_timestamp, { "contents": contents, "running_hash": running_hash, "sequence_number": sequence_number, }, - [chunk], transaction_id )
176-190:⚠️ Potential issue | 🔴 Critical
__str__references the removedself.chunksattribute — guaranteedAttributeErrorat runtime.Line 187 accesses
self.chunks, which no longer exists after this PR removes the attribute from__init__. Any code that prints or stringifies aTopicMessagewill crash.🐛 Proposed fix
def __str__(self) -> str: contents_str: str if isinstance(self.contents, bytes): contents_str = self.contents.decode("utf-8", errors="replace") else: contents_str = str(self.contents) return ( f"TopicMessage(" f"consensus_timestamp={self.consensus_timestamp}, " f"sequence_number={self.sequence_number}, " f"contents='{contents_str[:40]}{'...' if len(contents_str) > 40 else ''}', " - f"chunk_count={len(self.chunks)}, " f"transaction_id={self.transaction_id}" f")" )
45-56:⚠️ Potential issue | 🟡 MinorStale docstring still documents the removed
chunksparameter.Line 54 mentions
chunks (List[TopicMessageChunk])in theArgsblock, but that parameter no longer exists in the updated signature.🐛 Proposed fix
""" Args: consensus_timestamp (datetime): The final consensus timestamp. message_data (Dict[str, Union[bytes, int]]): Dict with required fields: { "contents": bytes, "running_hash": bytes, "sequence_number": int } - chunks (List[TopicMessageChunk]): All individual chunks that form this message. transaction_id (Optional[Transaction]): The transaction ID if available. """
Owner
Author
|
@coderabbitai configuration |
|
Configuration used: Path: .coderabbit.yaml CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
profile: assertive
request_changes_workflow: false
high_level_summary: false
high_level_summary_instructions: ''
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: false
review_details: false
commit_status: false
fail_commit_status: false
collapse_walkthrough: false
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: false
related_prs: false
suggested_labels: false
labeling_instructions: []
auto_apply_labels: false
suggested_reviewers: false
auto_assign_reviewers: false
in_progress_fortune: false
poem: false
enable_prompt_for_ai_agents: false
path_filters: []
path_instructions:
- path: .github/CODEOWNERS
instructions: >
You are acting as a senior maintainer reviewing the CODEOWNERS file
for the hiero-sdk-python repository. This file controls review
enforcement and repository governance.
Your role is to verify correctness, coverage, and organizational
alignment — not formatting preferences.
----------------------------------------------------------
REVIEW FOCUS 1 — TEAM SLUG CORRECTNESS (CRITICAL)
----------------------------------------------------------
All GitHub team references MUST use the exact, valid organization
team slugs that represent teams with write or higher permissions.
Flag as critical:
- Any non-existent team slugs
- Use of individual usernames
- Inclusion of teams that do NOT have write permissions
(eg `@hiero-ledger/hiero-sdk-python-triage`)
Expected teams commonly include:
- `@hiero-ledger/hiero-sdk-python-maintainers`
- `@hiero-ledger/hiero-sdk-python-committers`
- `@hiero-ledger/github-maintainers`
- `@hiero-ledger/tsc`
The following team MUST NOT appear because they do not have write
access:
- `@hiero-ledger/hiero-sdk-python-triage`
----------------------------------------------------------
REVIEW FOCUS 2 — DIRECTORY VS FILE MATCHING (HIGH IMPORTANCE)
----------------------------------------------------------
Directories that are intended to cover their contents MUST use /**
Flag directory cases like:
- /.github/workflows incorrect, use /.github/workflows/**
Specific files MUST remain specific to them and NOT use /**
- /.github/CODEOWNERS/** incorrect, use /.github/CODEOWNERS
Ensure rules actually match real repo assets.
----------------------------------------------------------
REVIEW FOCUS 3 — PRECEDENCE & ORDERING (HIGH IMPORTANCE)
----------------------------------------------------------
CODEOWNERS uses last-match-wins precedence.
Flag if rule ordering breaks CODEOWNERS precedence (last match wins):
- A broad rule (e.g. *) appears after more specific rules
- A broader path (e.g. /.github/**) appears after a more specific
subpath rule
(e.g. /.github/workflows/**)
- A sensitive path (e.g. /.github/workflows/**) is followed by a broader
rule that would override it
----------------------------------------------------------
REVIEW FOCUS 4 — REPOSITORY GOVERNANCE PROTECTION (HIGH IMPORTANCE)
----------------------------------------------------------
These paths must NOT be owned only by committers.
Ensure these include `@hiero-ledger/github-maintainers`:
- /.github/workflows/**
- /.github/scripts/**
- /.github/CODEOWNERS
- /CODEOWNERS (enables root-level protection)
Ensure packaging governance includes:
- /pyproject.toml → `@hiero-ledger/hiero-sdk-python-maintainers`,
`@hiero-ledger/github-maintainers`, `@hiero-ledger/tsc`
Flag missing governance coverage.
----------------------------------------------------------
REVIEW FOCUS 5 — SCOPE APPROPRIATENESS (MEDIUM IMPORTANCE)
----------------------------------------------------------
Committers may own source, tests, and documentation, but
governance and infrastructure files should be maintainer-level
or higher.
Flag if:
- TSC or GitHub maintainers are missing from critical governance files
----------------------------------------------------------
REVIEW FOCUS 6 — REDUNDANCY & DEAD RULES (MEDIUM IMPORTANCE)
----------------------------------------------------------
Identify rules that will never apply because a later rule overrides
them, or because the path does not exist.
Examples to flag:
- Duplicate patterns that are shadowed by broader ones, that do not
improve clarity
- Rules referencing directories or files not present in the repo
EXCEPTION:
The following paths are intentional future-protection rules and MUST
NOT be flagged as dead even if the file does not currently exist:
- /CODEOWNERS
----------------------------------------------------------
EXPLICIT NON-GOALS (CRITICAL IMPORTANCE)
----------------------------------------------------------
Do NOT:
- Suggest stylistic formatting changes
- Suggest reordering rules UNLESS precedence is incorrect
- Suggest adding reviewers or changing org governance policy
----------------------------------------------------------
FINAL OBJECTIVE
----------------------------------------------------------
Ensure the CODEOWNERS file:
- Uses valid GitHub team slugs
- Correctly matches repository paths
- Enforces intended review governance
- Has no precedence bugs that weaken protection
- path: src/hiero_sdk_python/tokens/**/*.py
instructions: >
You are acting as a senior maintainer reviewing token-related code
in the hiero-sdk-python project.
This includes:
- Token transaction classes (inherit from Transaction)
- Token data classes (e.g. TokenInfo)
- Token enums and status types
NOTE:
- Review focus levels indicate areas requiring careful verification.
- They do NOT imply severity or urgency.
- Only recommend fixes when behavior, safety, or API stability is
impacted.
Scope is STRICTLY LIMITED to:
- Changes under src/hiero_sdk_python/tokens/
- Their interaction with shared SDK base classes
----------------------------------------------------------
REVIEW FOCUS 1 — API STABILITY & BACKWARDS COMPATIBILITY
(CONTRACTUAL / HIGH SENSITIVITY)
----------------------------------------------------------
Token APIs are public and user-facing.
The following MUST remain stable:
- Public class names
- Method names and signatures
- Default values and behaviors
- Fluent setter chaining semantics
Flag any change that:
- Alters method signatures or return types
- Changes default behavior
- Renames or removes public attributes
If a breaking change is unavoidable:
- Require explicit deprecation warnings
- Require unit + integration tests
- Require inline comments explaining migration impact
----------------------------------------------------------
REVIEW FOCUS 2 — TRANSACTION BASE CLASS CONTRACT
----------------------------------------------------------
Applies to classes inheriting from Transaction.
REQUIRED STRUCTURE:
- Must call super().__init__() correctly
- Must implement:
* _build_proto_body()
* build_transaction_body()
- build_scheduled_body():
* may be less strict, but must be intentional and documented
- _get_method():
* MUST reference the correct Hedera service stub
* MUST align with protobuf service definitions
Subclasses MUST NOT:
- Override execution, retry, or backoff logic
- Manage gRPC deadlines manually
- Bypass Transaction lifecycle hooks
----------------------------------------------------------
REVIEW FOCUS 3 — PROTOBUF ALIGNMENT
----------------------------------------------------------
Token behavior MUST align with official Hedera protobuf definitions:
https://github.com/hashgraph/hedera-protobufs/tree/8c27786cec93abab974309074feaef9b48a695b7/services
Verify that:
- Protobuf field names match exactly
- Optional vs required fields are respected
- Defaults match protobuf expectations
- No silent divergence in semantics
Flag mismatches explicitly.
----------------------------------------------------------
REVIEW FOCUS 4 — VALIDATION & ERROR BEHAVIOR
----------------------------------------------------------
Validation rules:
- All required identifiers must be validated before proto construction
- Errors must be deterministic and user-readable
- Exceptions should be raised early and consistently
Avoid:
- Deferred runtime failures
- Ambiguous or generic error messages
- Silent no-op behavior
----------------------------------------------------------
REVIEW FOCUS 5 — IMPORT & HALLUCINATION SAFETY
----------------------------------------------------------
STRICT IMPORT RULES:
- All imports MUST exist in src/hiero_sdk_python/
- Validate paths against existing token modules or shared SDK code
- Do NOT assume helpers, enums, or utilities exist
Flag immediately:
- Non-existent modules
- Inferred helpers not present in the codebase
- Copy-paste artifacts from examples or other SDKs
----------------------------------------------------------
REVIEW FOCUS 6 — DATA CLASSES & ENUMS
----------------------------------------------------------
Applies to non-transaction token files.
Verify that:
- Enums map cleanly to protobuf or documented values
- No magic numbers or undocumented defaults exist
- Public classes are extensible and documented
- Convenience methods (__str__, __repr__) are present where useful
----------------------------------------------------------
REVIEW FOCUS 7 — TEST & EXAMPLE EXPECTATIONS
----------------------------------------------------------
Good to check whether:
- Unit tests exist for new or modified behavior
- Integration tests exist for new transactions
- Examples reference real, existing APIs
Missing coverage should be flagged clearly.
----------------------------------------------------------
REVIEW FOCUS 8 — EXPLICIT NON-GOALS
----------------------------------------------------------
Do NOT:
- Propose refactors unless correctness or safety is impacted
- Review unrelated SDK components
- Comment on formatting or lint-only issues
----------------------------------------------------------
FINAL OBJECTIVE
----------------------------------------------------------
Ensure token code is:
- Backward-compatible
- Transaction-lifecycle correct
- Protobuf-aligned
- Deterministic and user-safe
- path: examples/**/*
instructions: >
You are acting as a senior maintainer reviewing SDK examples. Your goal
is to ensure examples work verbatim for users who copy-paste them.
**Priority 1 - Correctness**:
- Verify transaction lifecycle chain (construction -> freeze_with ->
sign -> execute).
- Ensure `freeze_with(client)` is called BEFORE signing.
- Validate that methods referenced actually exist in the
`hiero_sdk_python` codebase.
- Ensure response validation checks `receipt.status` against
`ResponseCode` enums (e.g., `ResponseCode.SUCCESS`).
**Priority 2 - Transaction Lifecycle**:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit `.execute(client)` calls.
- Verify response property extraction (e.g., using `.token_id`,
`.account_id`, `.serial_numbers`).
- Ensure error handling uses `ResponseCode(receipt.status).name` for
clarity.
**Priority 3 - Naming & Clarity**:
- Enforce role-based naming: `operator_id`/`_key`,
`treasury_account_id`/`_key`, `receiver_id`/`_key`.
- Use `_id` suffix for AccountId and `_key` suffix for PrivateKey
variables.
- Validate negative examples explicitly check for failure codes (e.g.,
`TOKEN_HAS_NO_PAUSE_KEY`).
- Ensure logical top-to-bottom flow without ambiguity.
**Priority 4 - Consistency**:
- Verify standard patterns: `def main()`, `if __name__ == "__main__":`,
`load_dotenv()`.
- **IMPORT RULES**:
1. Accept both top-level imports (e.g., `from hiero_sdk_python import PrivateKey`) and fully qualified imports (e.g., `from hiero_sdk_python.crypto.private_key import PrivateKey`).
2. STRICTLY validate that the import path actually exists in the project structure. Compare against other files in `/examples` or your knowledge of the SDK file tree.
3. Flag hallucinations immediately (e.g., `hiero_sdk_python.keys` does not exist).
- Check for `try-except` blocks with `sys.exit(1)` for critical
failures.
**Priority 5 - User Experience**:
- Ensure comments explain SDK usage patterns (for users, not
contributors).
- Avoid nitpicking functional code.
- Suggest type hints or docstrings only if they significantly improve
clarity.
**Philosophy**:
- Examples are copied by users - prioritize explicitness over brevity.
- Avoid suggestions that `ruff` or linters would catch.
- Be concise, technical, and opinionated.
- Flag out-of-scope improvements as potential new issues rather than
blocking.
- path: tests/unit/**/*
instructions: >
You are acting as a senior maintainer reviewing unit tests for the
hiero-sdk-python project. Your goal is to ensure tests are extensive,
deterministic, and protect against breaking changes.
**CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically**:
- Tests must provide useful error messages when they fail for future
debugging.
- No `print()` statements - use assertions with descriptive messages.
- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
**PRIORITY 1 - Protect Against Breaking Changes**:
- Assert public attributes exist (e.g., `assert hasattr(obj,
'account_id')`).
- Assert return types where relevant (e.g., `assert isinstance(result,
AccountId)`).
- Assert fluent setters return `self` (e.g., `assert tx.set_memo("test")
is tx`).
- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation
behavior and test old behavior until removal.
**PRIORITY 2 - Constructor & Setter Behavior**:
- Test constructor behavior with valid inputs, edge cases, and invalid
inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
**PRIORITY 3 - Comprehensive Coverage**:
- Unit tests should be extensive - test even if we don't expect users to
use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the
contract.
**PRIORITY 4 - No Mocks for Non-Existent Modules**:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual `src/hiero_sdk_python`
structure.
- Mocks should only be used for external dependencies, not SDK
internals.
**PRIORITY 5 - Test Framework Philosophy**:
- Prefer repetitive but clear tests over abstracted helper functions.
- Some core functionality may warrant helper files (considered an
exception).
- Discourage custom helper functions; prefer pytest fixtures when shared
setup is needed.
- Prefer testing real functionality over mocked behavior.
**AVOID**:
- Linter or formatting feedback (leave that to ruff/pre-commit).
- Nitpicking minor stylistic issues unless they impact maintainability.
- Overly abstracted test helpers that obscure what's being tested.
**PHILOSOPHY**:
- Unit tests protect our future selves - be defensive and
forward-looking.
- Tests should be readable by SDK developers: clear names, brief
docstrings, key inline comments.
- When tests fail, we should immediately know what broke and why.
- path: tests/integration/**/*
instructions: >
You are acting as a senior maintainer reviewing integration tests for
the hiero-sdk-python project. Your goal is to ensure end-to-end tests
validate real network behavior safely and deterministically.
**CRITICAL PRINCIPLES - Safety & Diagnosability**:
- **Prioritize safety**: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not
hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just `SUCCESS`.
- Failure-path tests must assert specific `ResponseCode` values (e.g.,
`TOKEN_HAS_NO_PAUSE_KEY`).
**PRIORITY 1 - End-to-End Behavior**:
- Tests should be end-to-end: construct → freeze → sign → execute →
verify.
- Validate resulting balances, ownership, and state changes (not just
transaction success).
- Assert transaction receipts contain expected data (IDs, serial
numbers, etc.).
- Verify network state after operations (e.g., account balance changed,
token transferred).
**PRIORITY 2 - Test Structure & Maintainability**:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline
comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller
modules.
- Are helper functions good candidates for pytest fixtures or shared
utilities?
**PRIORITY 3 - Isolation & Cleanup**:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid
state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle
assumptions).
**PRIORITY 4 - Assertions & Coverage**:
- Do tests validate only success/failure, or also assert resulting
state?
- Suggest additional assertions that would strengthen correctness
(balances, allowances, ownership).
- Cover happy paths AND unhappy paths (e.g., invalid spender, revoked
allowance, insufficient balance).
- Avoid timing-based or flaky assumptions.
**PRIORITY 5 - Observability & Debugging**:
- Could structured logging or transaction metadata improve debugging?
- Suggest capturing allowance values, transaction IDs, and balances in
logs.
- Ensure error messages provide context for failure diagnosis.
**AVOID**:
- Linter or formatting feedback.
- Overly abstracted helpers that obscure what's being tested.
- Timing-dependent assertions (use explicit waits or retries if needed).
**PHILOSOPHY**:
- Integration tests validate real network behavior - they must be
reliable and safe.
- Tests should protect against regressions while being maintainable.
- When tests fail in CI, developers should immediately understand what
broke.
- Redundant setup code should be refactored, but clarity trumps
abstraction.
- path: docs/**
instructions: >
You are reviewing documentation for the Hiero Python SDK. These pages
serve both SDK users and SDK developers.
Priority 1 - Correctness (code, commands, links)
1. Verify code snippets conceptually run and match the current SDK API.
2. Check shell commands and workflow steps against actual project
tooling.
3. Validate URLs and cross-references; flag broken or misleading links.
Priority 2 - Clarity and completeness
1. Ensure each page states its purpose and expected outcome early.
2. Prefer concrete, step-wise explanations over vague descriptions.
3. Highlight missing prerequisites that would block a reader.
4. For larger gaps, suggest filing a follow-up issue instead of
blocking.
Priority 3 - Consistency and navigation
1. Encourage consistent terminology with the SDK and examples.
2. Check headings form a logical reading path.
3. Confirm each page makes clear which audience it serves.
PHILOSOPHY
- Treat docs as work-in-progress: optimize for correctness and clarity
over perfection.
- Keep feedback concise, action-oriented, and focused on reader success.
- Do not request large-scale restructures unless current structure
blocks understanding.
AVOID
- Avoid lint-style feedback on Markdown formatting or minor wording.
- Avoid proposing new conventions without clear benefit.
- Avoid turning every high-level gap into a blocker.
- path: docs/sdk_users/**
instructions: >
These documents are for SDK users who want to USE the Hiero Python SDK
quickly and correctly.
Priority 1 - High-level guidance
1. Ensure explanations are conceptual and point to `/examples` for
runnable code.
2. Check that required environment variables and network choices are
clearly stated.
Priority 2 - No hidden assumptions
1. Assume zero prior knowledge of this SDK and minimal Hedera
background.
2. Avoid requiring knowledge of repository layout or contribution
workflow.
PHILOSOPHY
- Keep explanations high-level and conceptual; defer runnable examples
to `/examples`.
- Focus on what users need to know before diving into code examples.
- path: docs/sdk_developers/**
instructions: >
These documents are for SDK developers and contributors, including
`docs/sdk_developers/training/**`.
Priority 1 - Workflow accuracy
1. Ensure contribution, branching, rebasing, signing (DCO, GPG), CI,
linting, and testing instructions match the repo.
2. Verify `git` and GitHub flows agree with CONTRIBUTING.md and current
practice.
3. Flag outdated references to scripts, directories, or configuration
files.
Priority 2 - Training flow
1. For training docs, ensure logical progression and clear
prerequisites.
2. Check that cross-links between training files are coherent.
PHILOSOPHY
- Treat these docs as a training ground for future maintainers and
regular contributors.
- Help readers move from "I cloned the repo" to "I can safely extend and
debug the SDK".
- Balance approachability for beginners with enough depth for experts.
- path: .github/workflows/**/*
instructions: >
Review workflows as security-sensitive infrastructure.
A good workflow is small, focused, and boring.
If a workflow is clever, generic, or overly flexible, it is a risk.
---------------------------------------------------------
PRIORITY 0 — ABSOLUTE REQUIREMENTS
---------------------------------------------------------
- All third-party actions MUST be pinned to full commit SHAs, similar to
other workflows.
- `permissions:` MUST be explicitly declared and minimally scoped.
- Workflows MUST behave safely when executed from forks.
- YAML MUST orchestrate steps, not implement business logic.
- Any workflow that mutates GitHub state MUST support dry-run mode.
- Dry-run behavior must be explicit and visible in logs.
- Workflows MUST NOT modify repository source code outside `.github/`.
---------------------------------------------------------
PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
---------------------------------------------------------
- The title of each workflow must be relevant, match similar naming
schemes, and match its script filename.
- Each workflow MUST have a single, clearly defined objective and SHOULD
document this in a top-level comment.
- Flag workflows that:
- Attempt to be generic “frameworks”
- Include speculative or future-facing logic
- Perform actions unrelated to the stated goal
- Over-abstraction and excess flexibility are maintenance risks.
---------------------------------------------------------
PRIORITY 2 — INPUT HARDENING
---------------------------------------------------------
- Treat ALL GitHub event data as potentially hostile input, including:
- issue titles, bodies, and comments
- labels, usernames, branch names
- Free-form user input MUST NOT be passed directly into:
- shell commands
- gh CLI arguments
- Node.js exec / spawn calls
- Require strict allowlists or exact string matches.
- Flag any use of:
- eval or bash -c
- backticks or $(...) with user-controlled input
---------------------------------------------------------
PRIORITY 3 — DRY-RUN & SAFE OPERATION
---------------------------------------------------------
- Workflows that mutate state MUST expose:
workflow_dispatch:
inputs:
dry_run:
default: "true"
- When dry_run=true, workflows MUST:
- Log dry mode is active
- Function on dry run: never post, comment, label, assign or edit
- Be easy to expand in the future
- Exit successfully
---------------------------------------------------------
PRIORITY 4 — SCRIPT EXTRACTION & CODE TRIM
---------------------------------------------------------
- YAML should orchestrate execution only.
- All non-trivial logic MUST live in:
- `.github/scripts/*.sh`
- `.github/scripts/*.js`
- Workflow filenames and their primary scripts SHOULD share a clear,
matching name.
- Scripts MUST remain:
- Purpose-built
- Trim and readable
- Easy to maintain
- Flag:
- Large `run: |` blocks
- Inline loops, conditionals, or API calls in YAML
- Overly generic helper scripts for narrow tasks
---------------------------------------------------------
PRIORITY 5 — API EFFICIENCY & DISCIPLINE
---------------------------------------------------------
- GitHub API usage must be intentional and minimal.
- Prefer:
- Aggregated queries over per-entity loops
- Server-side filtering over client-side iteration
- Reusing data already present in the event payload
- Pagination MUST:
- Be considered and justified
- Enforce hard upper bounds
- Flag:
- Repeated API calls inside loops
- Unbounded pagination
- Fetching data already available in context
---------------------------------------------------------
PRIORITY 6 — CONCURRENCY & IDEMPOTENCY
---------------------------------------------------------
- Workflows that mutate state MUST:
- Define a deterministic concurrency group
- Be safe under retries and parallel execution
- Duplicate prevention is REQUIRED:
- Marker-based comment detection
- Check-before-create logic for labels and assignments
- Assume workflows may:
- Run multiple times
- Be retried automatically
- Execute concurrently with other automations
- Workflows should avoid logic that duplicates or causes conflicts.
---------------------------------------------------------
PRIORITY 7 — PERMISSION CORRECTNESS
---------------------------------------------------------
- Requested permissions MUST exactly match behavior.
- Explicitly validate common cases:
- issues: write → comments, labels, assignments
- contents: read → repository checkout
- pull-requests: write → PR mutations
- Flag:
- Over-permissioned workflows
- Under-permissioned workflows that would fail at runtime
- Reliance on default permissions
---------------------------------------------------------
PRIORITY 8 — LOGGING & OPERABILITY
---------------------------------------------------------
- Logs should include, where applicable:
- repository
- workflow name
- issue or PR number
- triggering actor
- dry-run status
- decisions made (performed vs skipped)
- Avoid:
- Silent success or silent skips
- Raw payload dumps
- Any secret or token leakage
- path: .github/scripts/**/*.sh
instructions: |
Treat shell scripts as production-grade automation.
Scripts should be small, focused, and explicit.
Avoid “do-everything” or overly generic scripts.
- MUST use: `set -euo pipefail`
- MUST validate all required environment variables
- MUST defensively quote variables
- MUST validate all untrusted input
- MUST have bounded loops and pagination
- MUST support dry-run mode if state is mutated
- MUST log key decisions and early exits
- path: .github/scripts/**/*.js
instructions: |
Review JavaScript scripts as long-lived automation code.
Scripts must remain:
- Focused
- Readable
- Purpose-built
- All `context.payload` fields MUST be validated
- Free-form text MUST NOT be trusted
- Dynamic code execution is prohibited
- Avoid `child_process.exec`; prefer `execFile` if needed
- All async operations MUST be wrapped in try/catch
- Errors MUST include contextual metadata
- Duplicate API calls MUST be avoided
- Marker-based deduplication is required
- Scripts MUST NOT assume write access
- Permission failures MUST be handled gracefully
- path: src/hiero_sdk_python/query/**/*.py
instructions: >
You are acting as a senior maintainer reviewing the Query base class
and its subclasses for the hiero-sdk-python project.
NOTE:
- Review focus levels indicate areas that are important to check
carefully.
- They do NOT imply severity or urgency.
- Only recommend fixes when there is a clear behavioral regression.
Scope is STRICTLY LIMITED to:
- Changes to the base `Query` class
- Changes to existing `Query` subclasses
- Newly added `Query` subclasses
----------------------------------------------------------
REVIEW FOCUS 1 — QUERY SEMANTICS & PAYMENT BEHAVIOR
(CONTRACTUAL / HIGH SENSITIVITY)
----------------------------------------------------------
Queries do not reach consensus and use `QueryHeader` for payment and
responseType.
The following behaviors are contractual and must remain unchanged:
- `_is_payment_required()` semantics
- FREE vs PAID query classification
- COST_ANSWER vs ANSWER_ONLY behavior
- Whether a payment transaction is attached
Good to check and verify that changes do NOT:
- Alter FREE → PAID or PAID → FREE behavior
- Attach payment to COST_ANSWER queries
- Bypass `get_cost(client)` for paid queries
- Hardcode fees or override payment logic
----------------------------------------------------------
REVIEW FOCUS 2 — EXECUTION LIFECYCLE & BASE CLASS INTEGRITY
----------------------------------------------------------
All queries MUST:
- Use the base `Query` execution flow
- Delegate retries, backoff, and node selection to `_Executable`
- Call `_before_execute(client)` before `_execute(client)`
Subclasses MUST NOT:
- Override retry logic
- Implement custom node selection
- Manage gRPC deadlines manually
- Bypass `_Executable` state handling
Flag deviations for review; recommend fixes only if behavior changes.
----------------------------------------------------------
REVIEW FOCUS 3 — REQUEST CONSTRUCTION CONTRACT
----------------------------------------------------------
`_make_request()` MUST:
- Validate all required identifiers (accountId, tokenId, topicId, etc.)
- Call `_make_request_header()` exactly once
- Populate protobuf fields via `_to_proto()` helpers
- Avoid manual `QueryHeader` mutation
Subclasses MUST NOT:
- Set `responseType` directly
- Inject payment logic
- Rebuild headers manually
----------------------------------------------------------
REVIEW FOCUS 4 — RESPONSE EXTRACTION & DOMAIN MAPPING
----------------------------------------------------------
`_get_query_response()` MUST:
- Return the exact protobuf response field
- Perform NO data transformation
- Match the expected protobuf response type
`execute()` MUST NOT:
- Implement retries or error handling
- Modify payment or execution behavior
- Catch and suppress execution errors
----------------------------------------------------------
REVIEW FOCUS 5 — NEW SUBCLASS VALIDATION
----------------------------------------------------------
For newly added `Query` subclasses:
- Ensure they extend `Query` directly
- Verify required abstract methods are implemented
- Confirm payment semantics match the Hedera API
- Validate protobuf service and method correctness
- Ensure naming matches existing query patterns
Missing or incorrect semantics should be flagged clearly.
----------------------------------------------------------
REVIEW FOCUS 6 — REGRESSION & BEHAVIOR CHANGE DETECTION
----------------------------------------------------------
Good to check whether any change:
- Alters base `Query` behavior
- Changes default responseType handling
- Modifies `_make_request_header()` usage
- Alters `_get_method()` behavior
- Introduces side effects (logging, prints, stack traces)
- Changes error propagation behavior
Small changes should be flagged for verification
if they could affect execution flow or payment safety.
----------------------------------------------------------
REVIEW FOCUS 7 — EXPLICIT NON-GOALS
----------------------------------------------------------
Do NOT:
- Review query consumers
- Propose refactors unless correctness is impacted
- Comment on style, formatting, or naming unless misleading
----------------------------------------------------------
FINAL OBJECTIVE
----------------------------------------------------------
Ensure Query code remains:
- Backward-compatible
- Payment-safe
- Execution-consistent
- Strictly aligned with Hedera query semantics
- path: src/hiero_sdk_python/contract/**/*_query.py
instructions: >
You are acting as a senior maintainer reviewing the Query base class
and its subclasses for the hiero-sdk-python project.
NOTE:
- Review focus levels indicate areas that are important to check
carefully.
- They do NOT imply severity or urgency.
- Only recommend fixes when there is a clear behavioral regression.
Scope is STRICTLY LIMITED to:
- Changes to the base `Query` class
- Changes to existing `Query` subclasses
- Newly added `Query` subclasses
----------------------------------------------------------
REVIEW FOCUS 1 — QUERY SEMANTICS & PAYMENT BEHAVIOR
(CONTRACTUAL / HIGH SENSITIVITY)
----------------------------------------------------------
Queries do not reach consensus and use `QueryHeader` for payment and
responseType.
The following behaviors are contractual and must remain unchanged:
- `_is_payment_required()` semantics
- FREE vs PAID query classification
- COST_ANSWER vs ANSWER_ONLY behavior
- Whether a payment transaction is attached
Good to check and verify that changes do NOT:
- Alter FREE → PAID or PAID → FREE behavior
- Attach payment to COST_ANSWER queries
- Bypass `get_cost(client)` for paid queries
- Hardcode fees or override payment logic
----------------------------------------------------------
REVIEW FOCUS 2 — EXECUTION LIFECYCLE & BASE CLASS INTEGRITY
----------------------------------------------------------
All queries MUST:
- Use the base `Query` execution flow
- Delegate retries, backoff, and node selection to `_Executable`
- Call `_before_execute(client)` before `_execute(client)`
Subclasses MUST NOT:
- Override retry logic
- Implement custom node selection
- Manage gRPC deadlines manually
- Bypass `_Executable` state handling
Flag deviations for review; recommend fixes only if behavior changes.
----------------------------------------------------------
REVIEW FOCUS 3 — REQUEST CONSTRUCTION CONTRACT
----------------------------------------------------------
`_make_request()` MUST:
- Validate all required identifiers (accountId, tokenId, topicId, etc.)
- Call `_make_request_header()` exactly once
- Populate protobuf fields via `_to_proto()` helpers
- Avoid manual `QueryHeader` mutation
Subclasses MUST NOT:
- Set `responseType` directly
- Inject payment logic
- Rebuild headers manually
----------------------------------------------------------
REVIEW FOCUS 4 — RESPONSE EXTRACTION & DOMAIN MAPPING
----------------------------------------------------------
`_get_query_response()` MUST:
- Return the exact protobuf response field
- Perform NO data transformation
- Match the expected protobuf response type
`execute()` MUST NOT:
- Implement retries or error handling
- Modify payment or execution behavior
- Catch and suppress execution errors
----------------------------------------------------------
REVIEW FOCUS 5 — NEW SUBCLASS VALIDATION
----------------------------------------------------------
For newly added `Query` subclasses:
- Ensure they extend `Query` directly
- Verify required abstract methods are implemented
- Confirm payment semantics match the Hedera API
- Validate protobuf service and method correctness
- Ensure naming matches existing query patterns
Missing or incorrect semantics should be flagged clearly.
----------------------------------------------------------
REVIEW FOCUS 6 — REGRESSION & BEHAVIOR CHANGE DETECTION
----------------------------------------------------------
Good to check whether any change:
- Alters base `Query` behavior
- Changes default responseType handling
- Modifies `_make_request_header()` usage
- Alters `_get_method()` behavior
- Introduces side effects (logging, prints, stack traces)
- Changes error propagation behavior
Small changes should be flagged for verification
if they could affect execution flow or payment safety.
----------------------------------------------------------
REVIEW FOCUS 7 — EXPLICIT NON-GOALS
----------------------------------------------------------
Do NOT:
- Review query consumers
- Propose refactors unless correctness is impacted
- Comment on style, formatting, or naming unless misleading
----------------------------------------------------------
FINAL OBJECTIVE
----------------------------------------------------------
Ensure Query code remains:
- Backward-compatible
- Payment-safe
- Execution-consistent
- Strictly aligned with Hedera query semantics
- path: src/hiero_sdk_python/consensus/**/*.py
instructions: >
Review against protobuf schemas at:
https://github.com/hiero-ledger/sdk-collaboration-hub/tree/main/v3-sandbox/prototype/protobuf/src/main/proto
Ensure all message fields and types align with the latest definitions
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
description_keyword: ''
auto_incremental_review: true
auto_pause_after_reviewed_commits: 5
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
custom: []
pre_merge_checks:
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
trufflehog:
enabled: true
checkov:
enabled: true
tflint:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
fortitudeLint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
clang:
enabled: true
cppcheck:
enabled: true
opengrep:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
trivy:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
stylelint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
blinter:
enabled: true
psscriptanalyzer:
enabled: true
chat:
art: false
auto_reply: false
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
issue_enrichment:
auto_enrich:
enabled: false
planning:
enabled: true
auto_planning:
enabled: true
labels: []
labeling:
labeling_instructions: []
auto_apply_labels: false
|
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Removed tools section and added web_search option under knowledge_base. Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
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/consensus/topic_message.py (1)
38-43:⚠️ Potential issue | 🟡 MinorPR title vs. implementation mismatch:
chunksparameter is still present.The PR title states "Remove chunks parameter from
TopicMessageinitialization," butchunks: List[TopicMessageChunk]remains in the constructor signature (line 42) and is actively stored asself.chunks(line 61) and consumed in__str__and by the factory methods. If the removal is intentional, the parameter and all its usages need to be deleted; otherwise the PR description should be updated.
| message_data: Dict[str, Union[bytes, int]], | ||
| chunks: List[TopicMessageChunk], | ||
| transaction_id: Optional[TransactionId] = None, | ||
| transaction_id: Optional[str] = None, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional style: prefer str | None over Optional[str] for the new annotations.
Ruff UP007 warns against Optional[X] in favour of X | None for modern Python (3.10+). While the pre-existing annotations on lines 58–60 have the same pattern, the new Optional[str] annotations introduced in this PR are in scope for the same rule.
♻️ Proposed refactor
- transaction_id: Optional[str] = None,
+ transaction_id: str | None = None,- self.transaction_id: Optional[str] = transaction_id
+ self.transaction_id: str | None = transaction_id- transaction_id: Optional[str] = None
+ transaction_id: str | None = NoneApply the same pattern to the identical local variable in of_many (line 105).
Also applies to: 62-62, 75-75, 105-105
Comment on lines
+75
to
+81
| transaction_id: Optional[str] = None | ||
| if response.HasField("chunkInfo") and response.chunkInfo.HasField("initialTransactionID"): | ||
| transaction_id = TransactionId._from_proto(response.chunkInfo.initialTransactionID) | ||
| tx_id = response.chunkInfo.initialTransactionID | ||
| transaction_id = ( | ||
| f"{tx_id.shardNum}.{tx_id.realmNum}.{tx_id.accountNum}-" | ||
| f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos}" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/hiero_sdk_python/consensus/topic_message.py | head -130Repository: MonaaEid/hiero-sdk-python
Length of output: 6049
🏁 Script executed:
# Find TransactionID proto definition and related files
find . -type f -name "*.proto" | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Search for TransactionID definition in the codebase
rg -n "class TransactionID|message TransactionID" --type py --type proto -C 3Repository: MonaaEid/hiero-sdk-python
Length of output: 98
🏁 Script executed:
# Find all proto files in the repository
fd -e protoRepository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Look for TransactionID in Python generated proto files
find . -type f -name "*_pb2.py" | head -20Repository: MonaaEid/hiero-sdk-python
Length of output: 51
🏁 Script executed:
# Search for where TransactionID is imported/used
rg "TransactionID" --type py -C 2Repository: MonaaEid/hiero-sdk-python
Length of output: 11218
🏁 Script executed:
cat -n src/hiero_sdk_python/transaction/transaction_id.py | head -100Repository: MonaaEid/hiero-sdk-python
Length of output: 4871
🏁 Script executed:
cat -n examples/consensus/topic_message.py | head -80Repository: MonaaEid/hiero-sdk-python
Length of output: 2969
🏁 Script executed:
rg -n "__str__|def __repr__" src/hiero_sdk_python/transaction/transaction_id.py -A 3Repository: MonaaEid/hiero-sdk-python
Length of output: 191
🏁 Script executed:
sed -n '174,185p' src/hiero_sdk_python/transaction/transaction_id.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 280
🏁 Script executed:
sed -n '160,175p' src/hiero_sdk_python/transaction/transaction_id.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 562
🏁 Script executed:
rg -n "def to_string" src/hiero_sdk_python/transaction/transaction_id.py -A 5Repository: MonaaEid/hiero-sdk-python
Length of output: 324
🏁 Script executed:
sed -n '106,120p' src/hiero_sdk_python/transaction/transaction_id.pyRepository: MonaaEid/hiero-sdk-python
Length of output: 658
🏁 Script executed:
rg "accountID.shardNum|accountID.realmNum|accountID.accountNum" --type py -C 2 | head -40Repository: MonaaEid/hiero-sdk-python
Length of output: 2785
Fix field access path and transaction ID format separator in of_single() and of_many().
The code attempts to access tx_id.shardNum, tx_id.realmNum, and tx_id.accountNum directly on the TransactionID protobuf message. These fields do not exist on TransactionID; they exist on the nested AccountID sub-message (tx_id.accountID). This will cause an AttributeError at runtime.
Additionally, the transaction ID format uses - as separator, but the canonical Hedera format (as confirmed by the TransactionId.to_string() method) uses @ (e.g., 0.0.123@1234567890.500).
Proposed fix
- transaction_id = (
- f"{tx_id.shardNum}.{tx_id.realmNum}.{tx_id.accountNum}-"
- f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos}"
- )
+ transaction_id = (
+ f"{tx_id.accountID.shardNum}.{tx_id.accountID.realmNum}.{tx_id.accountID.accountNum}@"
+ f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos}"
+ )Apply the same fix to lines 118-121 in of_many().
Comment on lines
+78
to
+81
| transaction_id = ( | ||
| f"{tx_id.shardNum}.{tx_id.realmNum}.{tx_id.accountNum}-" | ||
| f"{tx_id.transactionValidStart.seconds}.{tx_id.transactionValidStart.nanos}" | ||
| ) |
There was a problem hiding this comment.
Transaction ID string uses - as separator; the canonical Hedera format requires @.
A Hedera transaction ID looks like 0.0.9401@1598924675.82525000, where 0.0.9401 is the fee-payer account ID and 1598924675.82525000 is the timestamp in seconds.nanoseconds. The canonical format is accountID@validStartTime.
Using - as the delimiter produces a non-standard ID that will not be recognised by mirror-node APIs, explorers, or other SDK components that parse transaction IDs. The fix shown above (in the field-path comment) already applies the correct @ separator.
Also applies to: 118-121
Comment on lines
+21
to
+23
| knowledge_base: | ||
| web_search: true | ||
|
|
There was a problem hiding this comment.
knowledge_base is incorrectly nested under reviews and web_search has the wrong type — web search will not be enabled.
Two schema violations:
knowledge_baseis not a valid property underreviews(additionalProperties: false). It must be a top-level key, peer ofreviewsandchat.web_searchmust be an object{ enabled: true }, not a bare booleantrue.
🐛 Proposed fix
Remove lines 21–23 from within reviews: and add a correctly-structured top-level block instead:
enable_prompt_for_ai_agents: false # Disable prompts for AI agents (spammy)
- knowledge_base:
- web_search: true
-
Then, below the chat: block (or at any top-level position outside reviews:):
+knowledge_base:
+ web_search:
+ enabled: true
+
chat:
art: false
auto_reply: false
Comment on lines
+804
to
+809
| ############################################# | ||
| - path: "src/hiero_sdk_python/consensus/**/*.py" | ||
| instructions: | | ||
| Review against protobuf schemas at: | ||
| https://github.com/hiero-ledger/sdk-collaboration-hub/tree/main/v3-sandbox/prototype/protobuf/src/main/proto | ||
| Ensure all message fields and types align with the latest definitions |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
canonical protobuf definitions Hedera Hiero consensus TopicMessage repository
💡 Result:
The canonical protobuf definitions for (Hedera/Hiero) mirror-node Consensus “topic messages” (what most SDKs surface as TopicMessage) live in the hashgraph/hedera-protobufs repo, in:
mirror/consensus_service.proto— defines:ConsensusTopicQueryConsensusTopicResponse(the wire shape corresponding to a “topic message”)service ConsensusService { rpc subscribeTopic(...) returns (stream ConsensusTopicResponse); }(raw.githubusercontent.com)
If you’re looking at Hiero-namespace SDKs, their generated ConsensusTopicResponse types are derived from these same protos (e.g., hiero-ledger/hiero-sdk-go exposes proto/mirror.ConsensusTopicResponse). (pkg.go.dev)
Citations:
- 1: https://raw.githubusercontent.com/hashgraph/hedera-protobufs/main/mirror/consensus_service.proto
- 2: https://pkg.go.dev/github.com/hiero-ledger/hiero-sdk-go/v2/proto/mirror?utm_source=openai
Update protobuf reference to the canonical source.
The referenced URL points to a prototype/sandbox repository. The canonical protobuf definitions for Hedera/Hiero consensus messages are in hashgraph/hedera-protobufs, specifically mirror/consensus_service.proto, which defines ConsensusTopicResponse. Update the instructions to reference the official repository instead of the sandbox prototype location.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. Quick Fix for CHANGELOG.md ConflictsIf your conflict is only in CHANGELOG.md, you can resolve it easily using the GitHub web editor:
For all other merge conflicts, please read: Thank you for contributing! |
|
Hi there! I'm the LinkedIssueBot.
Thank you, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.