Skip to content

P1: Fix silent data corruption bugs #333

Description

@ajianaz

Summary

Cora scan found 8+ bugs where code silently produces incorrect results instead of failing fast. These are more dangerous than crashes because users get wrong output without knowing.

Issues

Severity sort inverted (#42 — src/engine/rules/mod.rs:82)

Comment says critical-first sort, but implementation uses ascending a.severity.cmp(&b.severity). max_findings truncation drops the most important findings.

Fix: Sort by descending severity priority (map to numeric rank).

Security findings dropped on LLM failure (#45 — src/engine/review.rs:267)

Fallback condition only checks rule_findings and secrets_findings, ignoring security_findings. Scans producing only security scanner findings return LLM error instead of results.

Fix: Include security_findings in fallback condition.

HashMap nondeterministic hash (#52 — src/engine/debt_tracker.rs:205)

snapshot_filename hashes HashMap entries in native iteration order. Same content → different filenames across runs. Breaks deduplication.

Fix: Sort keys before hashing, use BTreeMap, or serialize to stable JSON.

Debt score trend calculation wrong (#53 — src/engine/debt_tracker.rs:421)

quality_score_avg uses overall average, not recent period. Trend comparison is meaningless.

Fix: Compute recent_avg_quality from recent slice and compare to previous_avg_quality.

Category trend change includes total in delta (#54 — src/engine/debt_tracker.rs:436)

count comes from all_categories (includes previous period), so change = total - previous is inflated.

Fix: Build separate recent category count map, compare recent vs previous.

Config precedence inverted (#92 — src/config/loader.rs:341)

Auto-detected provider preset overrides explicit config values, contradicting documented precedence.

Fix: Apply precedence: env vars > config > auto-detect > defaults.

context_chain config ignored (#93 — src/config/schema.rs:424)

ReviewSection parses context_chain but merge_into never applies it to Config.

Fix: Add handling in merge_into for r.context_chain.

Hook install overwrites existing hooks (#96 — src/hook/install.rs:27)

Implementation writes HOOK_TEMPLATE directly, replacing any pre-existing hook behavior.

Fix: Backup existing hook and compose wrapper, or append cora logic.

Affected files

  • src/engine/rules/mod.rs
  • src/engine/review.rs
  • src/engine/debt_tracker.rs
  • src/config/loader.rs
  • src/config/schema.rs
  • src/hook/install.rs

Effort estimate: 2-3 days

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions