Skip to content

Set markdown AI summary view mode to be default#15499

Open
InAnYan wants to merge 2 commits intoJabRef:mainfrom
InAnYan:feat/ai-markdown-summary
Open

Set markdown AI summary view mode to be default#15499
InAnYan wants to merge 2 commits intoJabRef:mainfrom
InAnYan:feat/ai-markdown-summary

Conversation

@InAnYan
Copy link
Copy Markdown
Member

@InAnYan InAnYan commented Apr 5, 2026

Related issues and pull requests

Addresses https://discourse.jabref.org/t/add-a-jabref-setting-that-remembers-the-ai-summary-markdown-checkbox-state-so-markdown-view-can-stay-enabled-by-default-instead-of-resetting-each-time/9078

PR Description

Steps to test

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • [.] I manually tested my changes in running JabRef (always required)
  • [.] I added JUnit tests for changes (if applicable)
  • [.] I added screenshots in the PR description (if change is visible to the user)
  • [.] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [.] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [.] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Set Markdown as default AI summary view mode

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Changes AI summary view mode default from plain text to Markdown
• Updates checkbox initialization to reflect Markdown as default view
• Improves user experience with richer formatted output by default
Diagram
flowchart LR
  A["Summary Component Initialization"] -->|"updateContent parameter"| B["Plain Text Mode<br/>false"]
  A -->|"Checkbox selected state"| C["Unchecked"]
  D["Updated Initialization"] -->|"updateContent parameter"| E["Markdown Mode<br/>true"]
  D -->|"Checkbox selected state"| F["Checked"]
  B -.->|"Change"| E
  C -.->|"Change"| F
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java ✨ Enhancement +1/-1

Set Markdown as default view mode

• Changed updateContent(false) to updateContent(true) in initialize method
• Switches default view mode from plain text to Markdown format
• Updated comment to reflect new default behavior

jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java


2. jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml ✨ Enhancement +1/-1

Initialize Markdown checkbox as selected

• Added selected="true" attribute to markdownCheckbox element
• Ensures checkbox is initially checked to match Markdown default mode
• Provides visual feedback that Markdown view is active by default

jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Apr 5, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Non-localized Markdown checkbox text📘 Rule violation ≡ Correctness
Description
The modified FXML line still uses hard-coded user-facing text Markdown instead of a localized
%... key. This breaks localization expectations and prevents translation of the UI label.
Code

jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[16]

+                <CheckBox fx:id="markdownCheckbox" mnemonicParsing="false" onAction="#onMarkdownToggle" text="Markdown" selected="true" />
Evidence
PR Compliance ID 20 requires all user-facing UI text to be localized (FXML strings should be
prefixed with %). The changed CheckBox keeps text="Markdown" as a hard-coded label on the
modified line.

AGENTS.md
jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[16-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `CheckBox` label uses a hard-coded user-facing string (`Markdown`) in FXML instead of a localized resource key.
## Issue Context
JabRef requires UI text in FXML to be localized by using `%<key>` and adding the key to the localization bundles.
## Fix Focus Areas
- jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[16-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Trivial updateContent(true) comment 📘 Rule violation ⚙ Maintainability
Description
The changed inline comment // Start in Markdown mode restates what the boolean argument implies
and adds low-value noise. This violates the guideline to avoid trivial comments in new/changed code.
Code

jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[47]

+        updateContent(true); // Start in Markdown mode
Evidence
PR Compliance ID 14 requires avoiding trivial comments that merely restate the code. The modified
line adds/updates an inline comment that repeats the intent already expressed by the call and UI
state change.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[47-47]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An inline comment restates the code behavior and does not add intent/why, increasing noise.
## Issue Context
Project guidelines ask to avoid trivial comments in changed code.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[47-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Unsanitized HTML rendered default 🐞 Bug ⛨ Security
Description
SummaryShowingComponent now initializes in Markdown mode and loads MarkdownFormatter output directly
into a WebView as HTML. MarkdownFormatter does not suppress/escape inline HTML, so any HTML present
in AI-generated summaries will be rendered as active HTML (e.g., links/images) instead of being
shown as text.
Code

jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[47]

+        updateContent(true); // Start in Markdown mode
Evidence
The component now starts in Markdown mode and, in that mode, renders summary.content() through
MarkdownFormatter and passes the result straight to WebViewEngine.loadContent(). MarkdownFormatter
is configured with an empty MutableDataSet and builds a flexmark HtmlRenderer without any explicit
HTML suppression/escaping, so inline HTML from the summary can pass through into the rendered HTML.

jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[44-70]
jablib/src/main/java/org/jabref/logic/layout/format/MarkdownFormatter.java[16-29]
jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryComponent.java[126-133]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
AI summary content is converted to HTML (Markdown -> HTML) and loaded into a JavaFX WebView without sanitization/escaping, which renders any embedded HTML markup as active HTML.
## Issue Context
- `SummaryShowingComponent` now defaults to Markdown rendering.
- `MarkdownFormatter` builds a flexmark parser/renderer with default options and returns raw HTML.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[44-70]
- jablib/src/main/java/org/jabref/logic/layout/format/MarkdownFormatter.java[16-29]
## Implementation direction
- Ensure rendered HTML is sanitized before calling `loadContent(...)` (e.g., clean the HTML output with a strict allowlist).
- Alternatively/also configure the Markdown renderer to escape or suppress inline HTML.
- Consider disabling JavaScript on this WebView instance if not required for summaries.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

4. Duplicate default view state 🐞 Bug ⚙ Maintainability
Description
The initial view mode is hardcoded in SummaryShowingComponent.initialize() while the FXML separately
sets the checkbox’s initial selected state. This duplicates the source of truth and can cause the
displayed mode and checkbox state to diverge when one side is changed.
Code

jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[16]

+                <CheckBox fx:id="markdownCheckbox" mnemonicParsing="false" onAction="#onMarkdownToggle" text="Markdown" selected="true" />
Evidence
Java initializes the content with a hardcoded boolean, while the FXML independently defines the
initial CheckBox state via selected="true"; there is no single authoritative source used in
initialize().

jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[44-49]
jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[14-17]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Default Markdown mode is specified twice (Java and FXML). This risks UI desync if one default is changed later.
## Issue Context
- Java calls `updateContent(true)`.
- FXML sets `selected="true"`.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java[44-49]
- jabgui/src/main/resources/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml[14-17]
## Implementation direction
- Make a single source of truth:
- Option A: In `initialize()`, call `updateContent(markdownCheckbox.isSelected())` and keep the default in FXML.
- Option B: Remove the FXML `selected` attribute and set the checkbox selection + content mode in Java.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Apr 5, 2026
@jabref-machine
Copy link
Copy Markdown
Collaborator

JUnit tests of jablib are failing. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Source Code Tests / Unit tests (pull_request)" and click on it.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Export\ to\ JSON=Export to JSON
Export\ to\ Markdown=Export to Markdown
Export\ chat\ history=Export chat history
Markdown=Makrdown
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Markdown dosn't need to be translated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh gosh
You are right

@jabref-machine
Copy link
Copy Markdown
Collaborator

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of - [x] (done), - [ ] (yet to be done) or - [/] (not applicable). Please adhere to our pull request template.

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

Labels

status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants