Skip to content

Fix /fix page#200

Open
Dru-429 wants to merge 3 commits into
ionfwsrijan:mainfrom
Dru-429:fix/fix-page
Open

Fix /fix page#200
Dru-429 wants to merge 3 commits into
ionfwsrijan:mainfrom
Dru-429:fix/fix-page

Conversation

@Dru-429

@Dru-429 Dru-429 commented Jun 25, 2026

Copy link
Copy Markdown

Before opening: make sure there is an issue tracking this work, and link it below. PRs without a linked issue may be closed without review.

Linked issue

Closes #185

What this PR does

This PR fixes bug #185 by implementing the frontend integration for the proposed fixes page and connecting it to the findings page and backend API. It enables full client-side handling for finding UUIDs via React Router state and introduces a fallback client-side visual diff generator when the backend returns null. The entire UI design has been kept strictly consistent with the existing layout throughout these updates.

Type of change

  • Bug fix
  • New feature
  • ML model / training pipeline
  • Refactor (no behaviour change)
  • Documentation
  • Tests only

ML tier (if applicable)

  • Tier 1 — Triage
  • Tier 2 — Predictive
  • Tier 3 — Autonomous
  • Not ML-related

Stack affected

  • Backend
  • Frontend
  • Both

Changes

Backend

Frontend

  • Link State Routing (findings.tsx): Passes selected finding UUIDs via React Router state on "Propose Fixes" and "Apply Fix" links.
  • Mapper (mappers.ts): Enhances UI mapper to handle both nested structures and flat database formats (file_path, line_number, scanner).
  • Dynamic Proposed Fixes (fix.tsx):
    • Loads active job_id and finds UUIDs directly from scan store and router state.
    • Normalizes ID prefixes (semgrep:, osv:, gitleaks:) before initiating the POST /fix API call.
    • Generates client-side line-by-line unified visual diffs as a fallback when diff = null.
    • Adds loading/error states and routes the apply action to the verification page with metadata.
    • Preserves all original page layouts and visual elements.

New dependencies

  • none

Database / schema changes

  • none

Testing

How did you test this?

Verified the frontend flow locally by selecting findings, navigating from the findings list to the proposed fixes page, and ensuring that router states pass correctly. Confirmed that the client-side line-by-line diff fallback works properly when diff = null, and that navigation onward to the verification page carries the correct metadata without throwing console errors.

Checklist

  • Tested locally end-to-end (upload ZIP or GitHub URL → scan → findings returned correctly)
  • New ML model falls back gracefully when model file is absent
  • No new console.error or unhandled Python exceptions introduced
  • Added or updated tests where applicable
  • requirements.txt / package.json updated if new dependencies added
  • New model files (.pkl, .pt, etc.) are gitignored, not committed

Anything reviewers should focus on

Please double-check the client-side line-by-line diff generator logic inside fix.tsx to verify that it handles edge-case code formatting correctly when the backend returns a null diff response.

Screenshots (if UI changed)

none

@github-actions

Copy link
Copy Markdown

🎉 Thank you @Dru-429 for submitting a Pull Request!

We're excited to review your contribution.

Before Review

✅ Ensure all CI checks pass
✅ Complete the PR template
✅ Link the related issue

Want faster reviews and contributor support?

Join our Discord community:

🔗 https://discord.gg/FcXuyw2Rs

Maintainers and mentors are active there and can help resolve blockers quickly.

Happy Contributing! 🚀

@arpit2006

Copy link
Copy Markdown
Collaborator

@Dru-429 , Just fix the template issue .
As soon as the fix is done , then the PR will go into mentor review stage!

@github-actions github-actions Bot added bug Something isn't working and removed needs-work Work needed labels Jun 26, 2026
@github-actions

Copy link
Copy Markdown

PR template check passed!

@arpit2006 this PR is ready for your review. 🚀

@arpit2006 arpit2006 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dru-429 !

PR Review Feedback

Requested Changes:

The client-side line-by-line diff generator fallback (generateDiff inside fix.tsx) has an algorithmic issue that incorrectly handles code insertions in the middle of a file. When a line is inserted, the logic fails to align the remaining lines, causing all subsequent lines of unchanged code to be flagged as deleted and added back, rather than remaining as context.

Example of Failure:

  • Original Code: ["A", "C"]
  • New Code: ["A", "B", "C"] (Single insertion of B in the middle)
  • Current Output:
      A
    - C
    + B
    + C
  • Expected Output:
      A
    + B
      C

Recommended Fix:

Replace the custom greedy loops in generateDiff with a standard Longest Common Subsequence (LCS) dynamic programming algorithm. This keeps diff generation stable and minimal:

function generateDiff(oldCode: string, newCode: string): DiffLine[] {
  const oldLines = (oldCode || "").split("\n");
  const newLines = (newCode || "").split("\n");
  
  if (!oldCode && !newCode) {
    return [
      { type: "context", content: "No code changes required for this remediation step." }
    ];
  }

  const m = oldLines.length;
  const n = newLines.length;
  
  // dp[i][j] stores the length of LCS of oldLines[0...i-1] and newLines[0...j-1]
  const dp: number[][] = Array.from({ length: m + 1 }, () => new Int32Array(n + 1));
  
  for (let i = 1; i <= m; i++) {
    for (let j = 1; j <= n; j++) {
      if (oldLines[i - 1] === newLines[j - 1]) {
        dp[i][j] = dp[i - 1][j - 1] + 1;
      } else {
        dp[i][j] = Math.max(dp[i - 1][j], dp[i][j - 1]);
      }
    }
  }
  
  const diffLines: DiffLine[] = [];
  let i = m;
  let j = n;
  
  // Backtrack to build the diff in reverse
  while (i > 0 || j > 0) {
    if (i > 0 && j > 0 && oldLines[i - 1] === newLines[j - 1]) {
      diffLines.push({
        type: "context",
        content: oldLines[i - 1],
        oldLine: i,
        newLine: j
      });
      i--;
      j--;
    } else if (j > 0 && (i === 0 || dp[i][j - 1] >= dp[i - 1][j])) {
      diffLines.push({
        type: "added",
        content: newLines[j - 1],
        newLine: j
      });
      j--;
    } else {
      diffLines.push({
        type: "removed",
        content: oldLines[i - 1],
        oldLine: i
      });
      i--;
    }
  }
  
  return diffLines.reverse();
}

Please do necessary changes then request review again!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Fix Page (/fix) Is Entirely Hardcoded Sample Data — No Backend Integration and Findings Selection Is Lost During Navigation

2 participants