Filter out large-table lint records from core rest API to fix LintHint being overwhelmed with spurious large-tables lint records
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Sbailey
	May 23 2023, 1:01 AM

Description

The LintHint gadget uses the core parsoid lint API to retrieve all lint errors for a specific page directly from Parsoid.
We need to suppress that API from returning 'large-tables' category lint records.
Setting the linter extension extension.json value to "none' priority has no impact on this core interface.

The first patch I am proposing filters out the 'large-table' records after Parsoid generates them, and is inelegant, but safe.
It might be better to pass an option to Parsoid such that the Parsoid lint code it does not look for and generate 'large-table' lint records when called from the core lint API with a special option disabling this lint.

The issue to be determined is who else besides LintHint is accessing this API and whether they want to see 'large-table' lint records or not.

In discussion with CScott it appears that Discussion Tools is also possibly using this API, but would also like the new category 'large-tables' to not be in the results.

Details

Subject	Repo	Branch	Lines +/-
Pass in linterOverrides, rather than filter after the fact	mediawiki/core	master	+34 -44
Bump wikimedia/parsoid to 0.20.0-a2	mediawiki/vendor	master	+279 -121
Allow callers to pass in linterOverrides	mediawiki/services/parsoid	master	+70 -50
Don't include hidden category counts in page info	mediawiki/extensions/Linter	master	+7 -2
Filter out all priority none lints from rest api	mediawiki/core	master	+48 -13
Filter out large-tables category lints from Parsoid REST API	mediawiki/core	master	+17 -0

Customize query in gerrit

Related Objects

Mentioned In: T334527: Allow registration of hidden lint rules
T336316: [Spike] Review web team Special linting rules
Mentioned Here: T336528: P99 performance of Linting went from 1-2s to 8-11 seconds
T336316: [Spike] Review web team Special linting rules
T336679: Improve large table lint handling of nested tables and tables with image tags

Event Timeline

Sbailey created this task.May 23 2023, 1:01 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 23 2023, 1:01 AM

Change 922177 had a related patch set uploaded (by Sbailey; author: Sbailey):

[mediawiki/core@master] WIP Filter out large-table lints from core test API

https://gerrit.wikimedia.org/r/922177

gerritbot added a project: Patch-For-Review.May 23 2023, 1:02 AM

Sbailey added a subscriber: ihurbain.May 23 2023, 2:41 PM

Sbailey updated the task description. (Show Details)May 23 2023, 2:53 PM

Sbailey added a subscriber: daniel.

The issue to be determined is who else besides LintHint is accessing this API and whether they want to see 'large-table' lint records or not.

This hints at removing any logic from that endpoint for filtering lints (as in the patch above) and making the default to not emit them. Then, add a configuration so that they only get emitted when we're linting as part of a full page parse that is going to be recorded by the linter extension.

Jdlrobson awarded a token.May 25 2023, 4:55 AM

In T337275#8875033, @Arlolra wrote:

The issue to be determined is who else besides LintHint is accessing this API and whether they want to see 'large-table' lint records or not.

This hints at removing any logic from that endpoint for filtering lints (as in the patch above) and making the default to not emit them. Then, add a configuration so that they only get emitted when we're linting as part of a full page parse that is going to be recorded by the linter extension.

I agree there is some need to have that configurable, and that we need to discuss/decide how and with what granularity - the above patch was mostly meant to be a stopgap to solve the immediate issue while we think about that.

I'm wondering whether we want:

a "Not-A-Lint"/"Internal" category that we'd indeed filter essentially everywhere except where we need it internally
an "allow" list of "allowed lints" to pass through the API
a "deny" list of "denied lints" to pass through the API
all or a combination of the above

I'm somewhat leaning towards the first solution at least, because it feels like it gives us the possibility to continue experimenting with the Linter without clients having to ever see it/adjust their tools to filter it out, but giving control to the users also feels like a Good Thing - so ideally we'd get ALL OF THAT, but that'll typically take a little bit more time.

For now, filtering in ParsoidHandler should help LintHint avoid being swamped with large-tables lints.
Next step I think is to implement inside Parsoid, a "internal" versus "exposed" or something like that flag that large-tables can use to hide these not WT errors, but useful diagnostic or research lints behind.

Considering T336316#8869809, T336528, and T336679, I think you should just stop emitting these lint errors altogether until it's refined and can be suppressed from turning up in the lint http api endpoint.

Hmm, nested tables create false positives, test coverage was inadequate.
Maybe we just drop a return at the start of the current large-tables lint code in Parsoid until this lint code gets rewritten?

What's the current timeline for having the lint hidden?

That (returning at the start) sounds fine as long as it's temporary. Web team would really like to start digging into the data here and working towards general solutions to this problem - it's important we fix this within the next 3 months and progress is currently blocked on having a working lint that we can iterate on that's hidden from editor workflows.

until this lint code gets rewritten?

Can you elaborate on what's being rewritten? Or do you mean fixing the hidden category?

Note we are hoping to have a fix for T336528 by the end of next week cc @Mabualruz who is point person while I'm out next week.

Jdlrobson mentioned this in T336316: [Spike] Review web team Special linting rules.May 26 2023, 9:26 AM

I think we should ship the large-tables lint suppressing code for the Parsoid rest API and then back it out once the replacement large-tables lint code is in production. This will help LintHint be usable again, later next week.

Change 922177 merged by jenkins-bot:

[mediawiki/core@master] Filter out large-tables category lints from Parsoid REST API

https://gerrit.wikimedia.org/r/922177

Maintenance_bot removed a project: Patch-For-Review.May 26 2023, 5:29 PM

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.11; 2023-05-30).May 26 2023, 6:00 PM

Sbailey moved this task from Backlog to Deployed on the MediaWiki-extensions-Linter board.May 31 2023, 5:58 PM

Arlolra moved this task from Deployed to Backlog on the MediaWiki-extensions-Linter board.Aug 21 2023, 9:01 PM

Arlolra triaged this task as Medium priority.Aug 23 2023, 6:45 PM

Arlolra moved this task from Backlog to Action API on the MediaWiki-extensions-Linter board.

Change 1010626 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/core@master] Filter out all priority none lints from rest api

https://gerrit.wikimedia.org/r/1010626