feat: improve spec decoder with extract_raw_section and extract_struc… - #386
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesContract specification extraction
Diagnostic context enrichment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 (2)
crates/core/src/spec/decoder.rs (2)
164-193: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winBroken
UdtErrorEnumV0arm: undefined identifiers and lost error population — won't compile.This arm iterates
err_enum.casesascase, but the body referencesfunc,func.doc,func.inputs,func.outputs, andfunc_name— none of which are in scope here (they belong to theFunctionV0arm above). Additionally, theContractFunctionliteral at lines 187-192 omits thereturn_type_defandparam_defsfields that the struct requires (see the correct construction at lines 155-162). As a result, this arm no longer builds/pushes anyContractErrorEntry, soerrorsstays empty — breakingresolve_error_codeand its downstream consumers (contract error resolution).🐛 Proposed fix
ScSpecEntry::UdtErrorEnumV0(err_enum) => { let enum_name = err_enum.name.to_string(); for case in err_enum.cases.iter() { let case_name = format!("{}::{}", enum_name, case.name); let doc = if case.doc.is_empty() { None } else { - Some(func.doc.to_string()) - }; - - let mut params = Vec::new(); - for input in func.inputs.iter() { - let param_name = input.name.to_string(); - let param_type = format_type_def(&input.type_); - params.push((param_name, param_type)); - } - - let return_type = if func.outputs.is_empty() { - "Void".to_string() - } else { - format_type_def(&func.outputs[0]) - }; - - functions.push(ContractFunction { - name: func_name, - params, - return_type, - doc, + Some(case.doc.to_string()) + }; + + errors.push(ContractErrorEntry { + code: case.value, + name: case_name, + doc, }); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/src/spec/decoder.rs` around lines 164 - 193, Replace the broken UdtErrorEnumV0 handling with error-entry construction using err_enum and each case, rather than FunctionV0 identifiers such as func or func_name. Populate and push the required ContractErrorEntry fields, including the case name, documentation, return_type_def, and param_defs, matching the established construction pattern in the FunctionV0 arm so errors is populated for resolve_error_code.
294-321: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
UdtStructV0arm: field-doc copy-paste bug, missingstructs.push, and an unmatched pattern — won't compile and drops all parsed structs.Two distinct problems in this arm:
- Line 309 sets each field's
field_docfromstruct_spec.docinstead offield.doc, so every field ends up with the struct's own doc rather than its own.- After the fields loop closes (line 317), there is no
structs.push(...)for the built struct —structsis never populated. Immediately after,_ => {}(318) andErr(_) => break(320) appear where the arm's closing brace and the outer match's default arm should be.entryhere is already an unwrappedScSpecEntry(fromwhile let Ok(entry) = ScSpecEntry::read_xdr(...)on line 124), which has noErrvariant, soErr(_) => breakcannot type-check against it.🐛 Proposed fix
ScSpecEntry::UdtStructV0(struct_spec) => { let struct_name = struct_spec.name.to_string(); let doc = if struct_spec.doc.is_empty() { None } else { Some(struct_spec.doc.to_string()) }; let mut fields = Vec::new(); for field in struct_spec.fields.iter() { let field_name = field.name.to_string(); let field_type = format_type_def(&field.type_); let field_doc = if field.doc.is_empty() { None } else { - Some(struct_spec.doc.to_string()) + Some(field.doc.to_string()) }; fields.push(ContractStructField { name: field_name, type_name: field_type, doc: field_doc, type_def: Some(field.type_.clone()), }); } - _ => {} - }, - Err(_) => break, + + structs.push(ContractStructDef { + name: struct_name, + fields, + doc, + }); + } + _ => {} } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/src/spec/decoder.rs` around lines 294 - 321, Fix the `ScSpecEntry::UdtStructV0` match arm in the decoder: derive each field’s `field_doc` from `field.doc`, construct and push the completed `ContractStruct` into `structs` after the fields loop, and restore the arm’s closing brace. Remove the invalid `Err(_) => break` pattern from this match over the already-unwrapped `entry`, preserving only the appropriate default arm.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/spec/decoder.rs`:
- Around line 405-454: Update the ContractStructField construction inside
extract_structs to populate the required type_def field with
Some(field.type_.clone()), matching the existing constructions in
decode_contract_spec and ensuring extracted struct fields retain their full type
definition.
---
Outside diff comments:
In `@crates/core/src/spec/decoder.rs`:
- Around line 164-193: Replace the broken UdtErrorEnumV0 handling with
error-entry construction using err_enum and each case, rather than FunctionV0
identifiers such as func or func_name. Populate and push the required
ContractErrorEntry fields, including the case name, documentation,
return_type_def, and param_defs, matching the established construction pattern
in the FunctionV0 arm so errors is populated for resolve_error_code.
- Around line 294-321: Fix the `ScSpecEntry::UdtStructV0` match arm in the
decoder: derive each field’s `field_doc` from `field.doc`, construct and push
the completed `ContractStruct` into `structs` after the fields loop, and restore
the arm’s closing brace. Remove the invalid `Err(_) => break` pattern from this
match over the already-unwrapped `entry`, preserving only the appropriate
default arm.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4f891d1-5baf-455f-9d52-21cd413af58f
📒 Files selected for processing (1)
crates/core/src/spec/decoder.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/decode/context.rs`:
- Around line 9-38: Update enrich_report and its extract_* helpers to derive
function_name, arguments, return_value, auth_signatures, and auth_entries
directly from the RPC payload’s envelopeXdr/resultXdr/resultMetaXdr or
transactionData/auth/results fields. Remove reliance on normalized JSON keys
that RPC responses do not provide, while preserving the existing
TransactionContext and report assignments.
- Around line 47-53: Update extract_arguments to convert each argument with
as_str() first, preserving the raw value for strings and falling back to
Value::to_string() for non-string values; add a test covering string arguments
and their unquoted output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 99146ee3-5e1f-462a-bfd3-202dd6e2bec5
📒 Files selected for processing (1)
crates/core/src/decode/context.rs
| pub fn enrich_report(report: &mut DiagnosticReport, tx_data: &serde_json::Value) -> GratResult<()> { | ||
| let tx_hash = tx_data | ||
| .get("hash") | ||
| .and_then(|h| h.as_str()) | ||
| .unwrap_or("unknown") | ||
| .to_string(); | ||
|
|
||
| let ledger_sequence = tx_data | ||
| .get("ledger") | ||
| .and_then(serde_json::Value::as_u64) | ||
| .unwrap_or(0) as u32; | ||
|
|
||
| let context = TransactionContext { | ||
| tx_hash, | ||
| ledger_sequence, | ||
| function_name: extract_function_name(tx_data), | ||
| arguments: extract_arguments(tx_data), | ||
| return_value: extract_return_value(tx_data), | ||
| fee: extract_fee_breakdown(tx_data), | ||
| resources: extract_resource_summary(tx_data), | ||
| }; | ||
|
|
||
| report.transaction_context = Some(context); | ||
|
|
||
| report.auth_signatures = extract_auth_signatures(tx_data); | ||
|
|
||
| report.auth_entries = extract_auth_entries(tx_data); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "enrich_report" -C5 --type=rust
rg -n '"functionName"|"arguments"|"returnValue"' --type=rust
rg -n '"auth"' -C3 --type=rustRepository: Toolbox-Lab/Grat
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File map\n'
git ls-files 'crates/core/src/decode/*' 'crates/core/src/*decode*' 'crates/core/src/*' | sed -n '1,200p'
printf '\n## Outline of target file\n'
ast-grep outline crates/core/src/decode/context.rs --view expanded || true
printf '\n## Relevant slices from target file\n'
sed -n '1,220p' crates/core/src/decode/context.rs
printf '\n## Search for extractor definitions and callers\n'
rg -n "fn extract_(function_name|arguments|return_value|auth_signatures|auth_entries)|enrich_report\\(" crates/core/src -S
printf '\n## Search for tx_data shape hints\n'
rg -n '"functionName"|"arguments"|"returnValue"|"auth"|"envelopeXdr"|"resultXdr"|"resultMetaXdr"|diagnostic' crates/core/src -SRepository: Toolbox-Lab/Grat
Length of output: 25563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## decode/diagnostic.rs slices\n'
sed -n '1,120p' crates/core/src/decode/diagnostic.rs
printf '\n---\n'
sed -n '360,430p' crates/core/src/decode/diagnostic.rs
printf '\n## rpc/client.rs response examples around transaction payloads\n'
sed -n '450,620p' crates/core/src/rpc/client.rs
printf '\n## context.rs tests around flat keys\n'
sed -n '220,430p' crates/core/src/decode/context.rsRepository: Toolbox-Lab/Grat
Length of output: 20710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## RPC response struct definitions\n'
rg -n "struct (GetTransactionResponse|SimulateTransactionResponse)|impl .*return_value_xdr|serde\\(rename" crates/core/src/rpc/client.rs -n -A40 -B20
printf '\n## Search for synthesized functionName/arguments/returnValue keys anywhere else\n'
rg -n '"functionName"|"arguments"|"returnValue"|function_name|return_value' crates/core/src -S
printf '\n## Caller context around enrich_report in decode/diagnostic.rs\n'
sed -n '1,80p' crates/core/src/decode/diagnostic.rs
sed -n '360,410p' crates/core/src/decode/diagnostic.rsRepository: Toolbox-Lab/Grat
Length of output: 15045
Derive these fields from the RPC payload
getTransaction only exposes envelopeXdr/resultXdr/resultMetaXdr, and simulateTransaction exposes transactionData/auth/results; nothing in this codebase populates functionName, arguments, or returnValue onto tx_data. As written, transaction_context.function_name, arguments, return_value, auth_signatures, and auth_entries stay empty unless the caller normalizes the JSON first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/core/src/decode/context.rs` around lines 9 - 38, Update enrich_report
and its extract_* helpers to derive function_name, arguments, return_value,
auth_signatures, and auth_entries directly from the RPC payload’s
envelopeXdr/resultXdr/resultMetaXdr or transactionData/auth/results fields.
Remove reliance on normalized JSON keys that RPC responses do not provide, while
preserving the existing TransactionContext and report assignments.
| fn extract_arguments(tx_data: &serde_json::Value) -> Vec<String> { | ||
| tx_data | ||
| .get("arguments") | ||
| .and_then(|a| a.as_array()) | ||
| .map(|args| args.iter().map(std::string::ToString::to_string).collect()) | ||
| .unwrap_or_default() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files crates/core/src/decode/context.rs crates/core/src/decode -z | tr '\0' '\n' | sed '/^$/d'
printf '\n== context.rs excerpt ==\n'
cat -n crates/core/src/decode/context.rs | sed -n '1,220p'
printf '\n== search for extract_arguments / enrich_report callers ==\n'
rg -n "extract_arguments|enrich_report\(" crates/core/src -S
printf '\n== search for functionName / arguments / returnValue usage ==\n'
rg -n '"functionName"|"arguments"|"returnValue"|"\bauth\b"' crates/core/src -SRepository: Toolbox-Lab/Grat
Length of output: 13025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
samples = [
"alice",
42,
True,
None,
{"k":"v"},
]
for x in samples:
print(type(x).__name__, json.dumps(x))
PYRepository: Toolbox-Lab/Grat
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== diagnostic.rs excerpt around enrich_report call ==\n'
cat -n crates/core/src/decode/diagnostic.rs | sed -n '1,460p' | sed -n '360,430p'
printf '\n== resource_analyzer.rs excerpt around enrich_report or tx_data shaping ==\n'
cat -n crates/core/src/decode/resource_analyzer.rs | sed -n '330,430p'
printf '\n== search for synthesized fields ==\n'
rg -n '"functionName"|\"returnValue\"|\"diagnosticEvents\"|\"resultXdr\"|\"envelopeXdr\"|\"resultMetaXdr\"|\"function_name\"|\"return_value\"|\"arguments\"' crates/core/src/decode crates/core/src/rpc -SRepository: Toolbox-Lab/Grat
Length of output: 10096
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== diagnostic.rs excerpt around enrich_report call ==\n'
cat -n crates/core/src/decode/diagnostic.rs | sed -n '360,430p'
printf '\n== resource_analyzer.rs excerpt around enrich_report or tx_data shaping ==\n'
cat -n crates/core/src/decode/resource_analyzer.rs | sed -n '330,430p'
printf '\n== search for synthesized fields ==\n'
rg -n '"functionName"|"returnValue"|"diagnosticEvents"|"resultXdr"|"envelopeXdr"|"resultMetaXdr"|"function_name"|"return_value"|"arguments"' crates/core/src/decode crates/core/src/rpc -SRepository: Toolbox-Lab/Grat
Length of output: 10096
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== diagnostic.rs top/mid excerpt ==\n'
cat -n crates/core/src/decode/diagnostic.rs | sed -n '1,260p'
printf '\n== caller references in diagnostic.rs ==\n'
rg -n "enrich_report\(&mut report, &tx_data\)|let tx_data|tx_data =" crates/core/src/decode/diagnostic.rs -n -S
printf '\n== any helper constructing functionName/arguments/returnValue ==\n'
rg -n '"functionName"|"arguments"|"returnValue"' crates/core/src -SRepository: Toolbox-Lab/Grat
Length of output: 10866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== diagnostic.rs top/mid excerpt ==\n'
cat -n crates/core/src/decode/diagnostic.rs | sed -n '1,260p'
printf '\n== caller references in diagnostic.rs ==\n'
rg -n "enrich_report\\(&mut report, &tx_data\\)|let tx_data|tx_data =" crates/core/src/decode/diagnostic.rs -n -S
printf '\n== any helper constructing functionName/arguments/returnValue ==\n'
rg -n '"functionName"|"arguments"|"returnValue"' crates/core/src -SRepository: Toolbox-Lab/Grat
Length of output: 10866
extract_arguments should unwrap string values before stringifying. Value::to_string() emits JSON text, so string args render with quotes in the report. Use as_str() first (fall back to to_string() for non-strings) and add a test for string arguments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/core/src/decode/context.rs` around lines 47 - 53, Update
extract_arguments to convert each argument with as_str() first, preserving the
raw value for strings and falling back to Value::to_string() for non-string
values; add a test covering string arguments and their unquoted output.
- Fix decode_contract_spec: UdtStructV0 now properly pushes to structs vec
- Fix UdtErrorEnumV0 arm: was referencing FunctionV0 variables (copy-paste bug)
- Fix stray '_ => {}' inside UdtStructV0 arm
- Fix field_doc using struct_spec.doc instead of field.doc
- Add SpecParser::extract_raw_structs returning Vec<ScSpecUdtStructV0>
- Improve SpecParser::extract_structs: includes type_def in struct fields
- Graceful handling: missing/malformed sections return empty vectors
- Add comprehensive tests for struct extraction
2 participants
…ts methods
closes #373
Summary by CodeRabbit