fix(quality): explain strict gate failures#72
Conversation
WalkthroughThe pull request enhances error reporting for quality-gate failures in the decompilation pipeline. When a quality gate fails, the error message now includes detailed diagnostic information: paths to quality.json and report.json artifacts, specific failure reasons, numeric metrics (placeholder IFs, unresolved control flow, indirect call ratio, disassembly ratio), and contextual notes based on input type and backend configuration. A new formatting helper function generates this comprehensive message, and a corresponding unit test validates the output for strict mode placeholder IF-count rejections. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
🧹 Nitpick comments (2)
crates/flutterdec-core/src/pipeline/runners/tests.rs (1)
31-83: LGTM — test coverage matches the formatter branches.The constructed
QualityReport/SymbolQualityCountsexercise each conditional branch informat_quality_gate_failure_message(failures non-empty, non-APK input, internal backend, all-placeholder symbols) and the assertions match the literal strings produced by the formatter (including the hardcoded--max-placeholder-ifs 999999example). The numeric inputs (placeholder_ifs=1691,indirect_call_ratio≈0.1256,placeholder=5394) faithfully reproduce the scenario from PR#71.One minor observation: the
loaded_adapter_kind = "dynamic_snapshot_string_model_v1"branch is exercised but not explicitly asserted. Optional — consider addingassert!(msg.contains("adapter kind is dynamic_snapshot_string_model_v1"));to lock that note in too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/flutterdec-core/src/pipeline/runners/tests.rs` around lines 31 - 83, The test exercises the branch that includes the loaded_adapter_kind string but doesn't assert it; update the test function quality_gate_failure_message_explains_strict_placeholder_rejection to add an assertion that msg contains the adapter-kind note emitted by format_quality_gate_failure_message (referencing the loaded adapter string "dynamic_snapshot_string_model_v1" and the message text produced by format_quality_gate_failure_message), e.g. add assert!(msg.contains("adapter kind is dynamic_snapshot_string_model_v1")); so the test explicitly locks that branch.crates/flutterdec-core/src/pipeline/runners.rs (1)
148-207: Diagnostic message reads well; consider deriving the example threshold fromopt.The composition is clear and each branch maps cleanly to the test. One optional improvement: the suggested relaxed flags are hardcoded constants (
999999,1.0,0.0). If you thread&DecompileOptions(or just the relevant gate values) into this helper, you could surface the current configured thresholds alongside the relaxed example, e.g. "current --max-placeholder-ifs N (observed M); for exploratory runs try …". That would make the hint more actionable when users have already customized gates. Not blocking — the static example is still useful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/flutterdec-core/src/pipeline/runners.rs` around lines 148 - 207, The diagnostic currently prints hardcoded example thresholds inside format_quality_gate_failure_message; change the function signature to accept the current gate config (e.g., &DecompileOptions or a small struct with max_placeholder_ifs, max_unresolved_cf, max_indirect_call_ratio, min_disassembly_ratio) and use those values when constructing the hint text so the message shows the user's current thresholds (refer to the DecompileOptions fields or new small GateValues struct) alongside observed metrics (use report.placeholder_ifs, report.unresolved_cf, report.indirect_call_ratio, report.disassembly_ratio) and then print the suggested relaxed flags with the example values — update all call sites that invoke format_quality_gate_failure_message to pass the options or gate values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/flutterdec-core/src/pipeline/runners.rs`:
- Around line 148-207: The diagnostic currently prints hardcoded example
thresholds inside format_quality_gate_failure_message; change the function
signature to accept the current gate config (e.g., &DecompileOptions or a small
struct with max_placeholder_ifs, max_unresolved_cf, max_indirect_call_ratio,
min_disassembly_ratio) and use those values when constructing the hint text so
the message shows the user's current thresholds (refer to the DecompileOptions
fields or new small GateValues struct) alongside observed metrics (use
report.placeholder_ifs, report.unresolved_cf, report.indirect_call_ratio,
report.disassembly_ratio) and then print the suggested relaxed flags with the
example values — update all call sites that invoke
format_quality_gate_failure_message to pass the options or gate values.
In `@crates/flutterdec-core/src/pipeline/runners/tests.rs`:
- Around line 31-83: The test exercises the branch that includes the
loaded_adapter_kind string but doesn't assert it; update the test function
quality_gate_failure_message_explains_strict_placeholder_rejection to add an
assertion that msg contains the adapter-kind note emitted by
format_quality_gate_failure_message (referencing the loaded adapter string
"dynamic_snapshot_string_model_v1" and the message text produced by
format_quality_gate_failure_message), e.g. add assert!(msg.contains("adapter
kind is dynamic_snapshot_string_model_v1")); so the test explicitly locks that
branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 448af212-7f71-46ce-b50b-854d01078d48
📒 Files selected for processing (2)
crates/flutterdec-core/src/pipeline/runners.rscrates/flutterdec-core/src/pipeline/runners/tests.rs
1 participant
Summary
Testing
origin/mainand this branch both produce the same quality outcome on that input:placeholder_ifs = 1691indirect_call_ratio = 0.12557602191154899disassembly_ratio = 1.0placeholder if-count exceeded threshold