Skip to content

Aggregate linter messages belonging to the same category#90

Open
geritwagner wants to merge 1 commit into
mainfrom
aggregate_linter_messages
Open

Aggregate linter messages belonging to the same category#90
geritwagner wants to merge 1 commit into
mainfrom
aggregate_linter_messages

Conversation

@geritwagner

Copy link
Copy Markdown
Contributor

No description provided.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3cc9605bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread search_query/linter.py
parser = _run_parser(search_string, platform=platform, field_general=field_general)

return parser.linter.messages # type: ignore
return aggregate_linter_messages(parser.linter.messages) # type: ignore

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Skip aggregation for list-parser message maps

lint_query_string now unconditionally calls aggregate_linter_messages(parser.linter.messages), but _run_parser routes list-style inputs (queries containing "1.") to LIST_PARSERS, where parser.linter.messages is a dict keyed by list line number rather than a list of message dicts. Iterating that dict in aggregate_linter_messages yields integers, causing an AttributeError ('int' object has no attribute 'get') for list queries with any linter output (e.g., typical WoS/PubMed list searches), so this API path now fails at runtime instead of returning messages.

Useful? React with 👍 / 👎.

self.linter.validate_query_tree(query)
self.linter.check_status()

self.linter.messages = aggregate_linter_messages(self.linter.messages)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep EBSCO messages unaggregated before node assignment

Aggregating inside EBSCOParser.parse collapses same-category messages from different positions into one message before EBSCOListParser calls assign_linter_messages, but that assignment logic maps each message to a single list node using only the first position. As a result, in list queries where the same warning appears on multiple lines, only the first matching line keeps the warning and other affected lines lose it, which regresses per-line diagnostics for EBSCO list parsing.

Useful? React with 👍 / 👎.

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.

1 participant