PR: konveyor-ecosystem/semver-analyzer#17
Title: fix: co-generate fix guidance entries with rules to fix alignment bug
Branch: fix/fix-guidance-alignment-bug
Files changed: 3 (crates/ts/src/konveyor.rs, crates/ts/src/konveyor_v2.rs, src/main.rs)
Review date: 2026-05-07
This PR fixes the catastrophic alignment bug in fix-guidance.yaml where 99.8% of entries had mismatched ruleIDs and descriptions. The approach is sound: instead of generating guidance entries after-the-fact via positional indexing, it co-generates them alongside rules so pairing is correct by construction.
Verdict: The alignment fix is correct and should be merged. However, it has a coverage gap that should be addressed — either in this PR or as an explicit follow-up.
The old generate_fix_guidance() function walked through the report changes using positional indexing (rule_idx++) to pair with rules:
// OLD (broken): positional indexing into a reordered array
for api_change in &file_changes.breaking_api_changes {
if rule_idx < rules.len() {
let fix = api_change_to_fix(api_change, ..., &rules[rule_idx].rule_id, ...);
rule_idx += 1;
}
}This broke because rules had been consolidated (reordered, deduplicated), extended with dep-update rules, and augmented with sd_rules — none of which matched the report's iteration order.
The PR replaces this with co-generation inside generate_rules(), building a HashMap<String, FixGuidanceEntry> keyed by rule_id:
// NEW (correct): co-generated alongside the rule, keyed by rule_id
if let Some(primary) = new_rules.first() {
let entry = api_change_to_fix(api_change, file_changes, &primary.rule_id, file_pattern);
guidance_map.insert(primary.rule_id.clone(), entry);
}The HashMap approach is immune to ordering bugs — the key-value association is intrinsic.
When rules are consolidated (multiple rules merged into one), the PR correctly remaps guidance entries via id_mapping:
for (old_id, new_id) in &id_mapping {
if old_id != new_id {
if let Some(entry) = guidance_map.remove(old_id) {
guidance_map.entry(new_id.clone()).or_insert(entry);
}
}
}This ensures consolidated rules keep their guidance, using or_insert to prefer the first entry if multiple rules consolidate into one.
After all rules are assembled, the PR filters the guidance map to only include surviving rule IDs:
let surviving_ids: HashSet<String> = all_rules.iter().map(|r| r.rule_id.clone()).collect();
guidance_map.retain(|id, _| surviving_ids.contains(id));This correctly removes guidance for rules that were suppressed by v2 SD rules.
All 55+ test call sites updated to destructure the new return type. Existing guidance-specific tests still pass with the new API.
The test-impact query pattern fixes (^getByRole$ → (^|\.)getByRole$) and getAttribute pattern improvements are correct bug fixes for test-detection rules.
The PR only generates guidance entries for rules produced by generate_rules() in konveyor.rs. This covers:
- API changes (prop removed, renamed, type changed) ✅
- Behavioral changes ✅
- Manifest/dependency changes ✅
- P0C component-level changes (new in this PR) ✅
It does not generate guidance for rules produced by generate_sd_rules() in konveyor_v2.rs:
| Rule category | Prefix | Count (PF5→6) | Guidance? |
|---|---|---|---|
| Composition/hierarchy | sd-cf-* |
~180 | No |
| Prop value changes | sd-prop-value-* |
~73 | No |
| Component removal | sd-composition-* |
~18 | No |
| Deprecation moves | sd-deprecated-* |
~3 | No |
| Children→prop | sd-child-to-prop-* |
~1 | No |
| Prop→children | sd-prop-to-children-* |
~2 | No |
| Context usage | sd-context-* |
~10 | No |
| New siblings | sd-new-sibling-* |
varies | No |
| CSS class renames | semver-css-class-rename-* |
~2,024 | No |
| CSS class removed | semver-css-class-removed-* |
~445 | No |
| Consumer CSS renames | semver-consumer-css-* |
~232 | No |
| Dep-update rules | semver-dep-update-* |
varies | No |
The old generate_fix_guidance() also did not correctly generate entries for sd_rules, CSS rules, or dep-update rules. The old code iterated over report.changes and report.manifest_changes — neither of which contains sd_rules data. The sd_rules ruleIDs appeared in the old fix-guidance.yaml only because the positional indexing bug caused rule_idx to walk past the API/behavioral/manifest entries and into the sd_rules portion of all_rules, pairing their ruleIDs with unrelated API change descriptions.
In other words:
- Old file: sd_rules ruleIDs present but paired with wrong descriptions (actively harmful)
- New file: sd_rules ruleIDs absent (neutral — no guidance rather than wrong guidance)
- Neither ever produced correct guidance for sd_rules
So this PR doesn't make the sd_rules coverage worse — it was always zero. The difference is that the old file masked this gap by including sd_rules ruleIDs with garbage data.
These sd_rules include some of the most important migration guidance:
sd-prop-value-pagesection-variant-light— mapsvariant="light"to its replacement (benchmark TC062, TC066)sd-child-to-prop-emptystate-emptystateicon-to-icon— EmptyState icon migration (benchmark TC026, TC027)sd-prop-to-children-modal-footer-to-modalfooter— Modal restructuring (benchmark TC048)sd-deprecated-moved-chip-to-deprecated— Chip deprecation (benchmark TC012, TC013)sd-cf-masthead-brand-in-main— Masthead structural requirements (benchmark TC042, TC044)
These rules DO have entries in fix-strategies.json (which is correct and keyed by ruleID). So the LLM has the machine-readable strategy data. What's missing is the human-readable fix_description text that fix-guidance.yaml would provide.
Our evaluation data (3 runs, scores 54→51→50) suggests fix-strategies.json is the primary driver of LLM behavior, so this gap has limited practical impact. But it should still be addressed.
None — the PR should be merged as-is. The alignment fix is correct and important.
In src/main.rs, after generate_sd_rules() produces sd_rules, generate guidance entries and add them to guidance_map:
// After sd_rules are generated
for rule in &sd_rules {
if !guidance_map.contains_key(&rule.rule_id) {
guidance_map.insert(rule.rule_id.clone(), FixGuidanceEntry {
rule_id: rule.rule_id.clone(),
strategy: infer_strategy_from_labels(&rule.labels),
confidence: infer_confidence_from_labels(&rule.labels),
source: FixSource::Pattern,
symbol: extract_symbol_from_rule(&rule),
file: extract_file_from_rule(&rule),
fix_description: rule.description.clone(),
before: None,
after: None,
search_pattern: extract_search_pattern(&rule),
replacement: None,
});
}
}Alternatively, modify generate_sd_rules() in konveyor_v2.rs to co-produce guidance entries the same way this PR modified generate_rules() in konveyor.rs. This would be more consistent with the co-generation pattern established by this PR.
The sd_rules already contain description and message fields with good human-readable text. The guidance entries can be derived from these.
Same pattern — generate_dep_update_rules() returns rules that should also have guidance entries.
The 2,700+ CSS rules (class renames, removals, consumer CSS) are high-volume but low-value for fix-guidance.yaml. Most are mechanical pf-v5- → pf-v6- prefix changes already covered by CssVariablePrefix strategies in fix-strategies.json. Including them would add bulk without much signal. Consider including only non-trivial CSS rules (logical property renames, class removals).
The existing tests verify guidance generation for API changes, behavioral changes, and manifest changes. Consider adding a test that verifies guidance is generated for P0C component-level changes (the new code path added in this PR at line ~1889).
55+ test call sites use _guidance_map (with underscore prefix indicating unused). This is fine for tests that don't exercise guidance, but the naming convention makes it easy to miss when a test should be checking guidance. Not a blocking issue.
let fixes: Vec<FixGuidanceEntry> = guidance_map.into_values().collect();HashMap iteration order is non-deterministic. This means the YAML output order changes between runs, which makes diffing harder. Consider sorting by rule_id before collecting:
let mut fixes: Vec<FixGuidanceEntry> = guidance_map.into_values().collect();
fixes.sort_by(|a, b| a.rule_id.cmp(&b.rule_id));We ran the benchmark with the fix-guidance.yaml produced by this PR. Results:
| Metric | Before PR (broken) | After PR (fixed) |
|---|---|---|
| Fully correct (3/3) | 51/85 | 50/85 |
| Worse than pf-codemods | 23 | 19 (-4) |
| Better than pf-codemods | 11 | 13 (+2) |
| Equal to pf-codemods | 32 | 34 (+2) |
Key improvements from the fix:
- TC001, TC003 (Accordion): 1→3 — isExpanded migration now works
- TC045 (MenuItemAction): 1→3 — tool no longer over-modifies
- TC080 (Tokens): 2→3 — token renames now correct
Some 3→2 regressions where the LLM is now more confident and overshoots — expected behavior when the LLM stops receiving conflicting signals. These regressions are addressable by fixing wrong entries in fix-strategies.json (a separate issue).
Bottom line: The fix is net positive for migration quality. The vs-codemods comparison improved meaningfully.
| File | Purpose |
|---|---|
HANDOFF_fix_guidance_troubleshooting.md |
Full history of the fix-guidance.yaml investigation |
results/2026-05-05-semver-goose-050526-1644/fix-guidance-alignment-bug.md |
Original proof of the alignment bug |
results/2026-05-06-semver-goose-050626-2037/fix-guidance-impact-analysis.md |
Before/after evaluation with this PR's output |
results/2026-05-05-semver-goose-050526-1644/recommendations.md |
Full prioritized action plan |