Skip to content

fix: show findings once and refresh the open rule detail after a Rule Editor save - #378

Open
Abhiram Shaji (abhiramshaji-cyber) wants to merge 2 commits into
microsoft:mainfrom
abhiramshaji-cyber:fix/antipatterns-findings-rerender
Open

Abhiram Shaji (abhiramshaji-cyber) wants to merge 2 commits into
microsoft:mainfrom
abhiramshaji-cyber:fix/antipatterns-findings-rerender

Conversation

@abhiramshaji-cyber

@abhiramshaji-cyber Abhiram Shaji (abhiramshaji-cyber) commented Nov 14, 2026 •

Copy link
Copy Markdown

Description

Saving a rule made the findings list repeat every practice group and left the open rule detail out of date, and this PR shows each group once plus the saved rule.

Hi there, hope you're all doing well! Thanks a lot Sanjay Singh (@san360) for such a clear report. The line numbers and the 4, 8, 12 count from your probe made this really quick to track down. I'd love any feedback on whether this is the shape you want.

What this code is for

renderAntiPatterns in src/webview/page-antipatterns.ts draws the whole page into one container: the score cards, the findings under #apDetails, and the Rules tab with its rule detail panel. It runs when the page is opened, and again into the same container after a Rule Editor save, through the callback passed to wireRuleEditorModal.

Before this PR

A user opens Rules, clicks a rule, clicks Edit Source, then Save.

  1. saveRule answers and the editor calls renderAntiPatterns(container, currentFilter) again.
  2. Preact keeps the existing #apDetails element, with the sections added last time still inside it.
  3. renderFindings builds 4 new <details> sections and adds each with detailsContainer.appendChild(section) (this line on main).
  4. #apDetails now holds 8 sections. A second save makes it 12.
  5. #rule-detail-panel stays visible because only showRuleDetail writes its content, and nothing calls it again, so the detail still shows the rule from before the save.

After this PR

Same user, same Save.

  1. Same as above through step 2.
  2. renderFindings collects the 4 sections in an array and calls detailsContainer.replaceChildren(...sections), so whatever was in #apDetails is dropped.
  3. #apDetails holds 4 sections after every save.
  4. showRuleDetail now records the shown rule in panel.dataset.ruleId. At the end of renderAntiPatterns, if that is set, the page calls showRuleDetail again with the rules it just fetched, so the detail shows the saved values.
  5. If that rule is no longer in the list, showRuleDetail calls hideRuleDetail and the user is back on the rule list.
flowchart LR

classDef box fill:#eef1ff,stroke:#4c5bd4,color:#111827
classDef bad fill:#ffd6d6,stroke:#cc0000,color:#111827,stroke-width:3px
classDef good fill:#c8f0c8,stroke:#008800,color:#111827,stroke-width:3px

subgraph BEFORE["BEFORE: 4, then 8, then 12 sections"]
direction TB
  B1["Save in the Rule Editor"] --> B2["renderAntiPatterns"]
  B2 --> B3["renderFindings builds 4 sections"]
  B3 --> B4{"appendChild into apDetails"}
  B4 -->|"old sections kept"| B5["8 sections, detail shows old values"]
  B4 -->|"first open of the page"| B6["4 sections"]
end

subgraph AFTER["AFTER: 4 sections on every save"]
direction TB
  A1["Save in the Rule Editor"] --> A2["renderAntiPatterns"]
  A2 --> A3["renderFindings builds 4 sections"]
  A3 --> A4{"THE CHANGE: replaceChildren on apDetails"}
  A4 -->|"every save"| A5["4 sections, detail shows the saved rule"]
  A4 -->|"first open of the page"| A6["4 sections, same as today"]
end

B1 ~~~ A1

class B1,B2,B3,B6 box
class A1,A2,A3,A6 box
class B4,B5 bad
class A4,A5 good

linkStyle 3 stroke:#cc0000,stroke-width:5px
linkStyle 8 stroke:#008800,stroke-width:5px

style BEFORE fill:#fff5f5,stroke:#cc0000,color:#111827
style AFTER fill:#f3fff3,stroke:#008800,color:#111827
Loading

renderFindings on main and on this branch

renderFindings on main (left) and on this branch (right).

The change

  • The fix for the doubled findings: renderFindings ends with detailsContainer.replaceChildren(...sections) instead of calling appendChild per section. It always builds all 4 groups, including ones with no findings, so this covers the first open, every save, and empty data the same way.
  • The fix for the old detail: showRuleDetail sets panel.dataset.ruleId, the Back button and a missing rule both go through a new hideRuleDetail that clears it, and renderAntiPatterns shows the recorded rule again after it renders.
  • Opening the page from the sidebar or changing the date filter already clears the container in renderPage, so nothing changes there.

What changes for a user

  • One section per practice group after any number of saves.
  • An open rule detail shows the saved rule after Save, or goes back to the list if that rule is gone.
  • Opening the page, the tabs, the Back button and the Why? buttons work as before.
  • The findings sections are rebuilt on each save, so one that was expanded comes back collapsed, the same as after a reload.

One question: for the open detail I went with showing the saved rule again rather than closing it, since the issue mentions both. Would you prefer it to close instead? Happy to switch it.

Related Issues

Fixes #352

Checklist

  • npm run check passes (typecheck + lint + spellcheck + knip + tests). Typecheck, lint, spellcheck, knip and lockfile lint pass. In the tests, only the 2 src/core/image-data.test.ts cases fail on my Mac, and they fail on unchanged main too (/var vs /private/var paths). tests/e2e/antipatterns.spec.ts passes (20 of 20).
  • Changes are covered by tests (if applicable). New cases in page-antipatterns-editor-wiring.test.ts: one section per group after 1 and 3 saves, the open detail showing the saved rule, and the detail closing when its rule is gone. All 4 fail on main.
  • Documentation updated (if applicable). Not needed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After a Rule Editor save, Anti-Patterns findings show twice and the open rule detail keeps its old values

1 participant