Keep right QA

[thomas-hervey]

After the recent introduction of notes in iD, I've been discussing with @kamicut and @bhousel about the future of QA tools. A new QA section under "Map Data" could be used to display QA issues, such as KeepRight, a data consistency check that automatically identifies issues.

[bhousel]

So excited to try this out!! @thomas-hervey I know you've been working on it the past few days, but it looks like tests are passing again. Let me know if it's ready!

[thomas-hervey]

@bhousel, go ahead and test it out. I will be pushing small updates, but at this point it's ready for checking and cleaning.

[magol]

@thomas-hervey
I am looking forward to this being ready. How is the status of your job? Is it possible to test it already?

[thomas-hervey]

@magol thanks for your interest. I have been on the road so my work has been intermittent over the last weeks. I am going to work on it a bit more today and I hope to get feedback from @bhousel. However, feel free to checkout the branch and play around with it.

[tordans]

Hi @thomas-hervey can you give me a hand testing this? I cannot find the new feature in my dev editor.
What I did:

1. follow https://github.com/openstreetmap/iD#installation
2. checkout keep-right_QA
3. run the rpm commands from 1 again
4. open http://localhost:8080/#background=Berlin-2018&disable_features=boundaries&map=17.00/52.51669/13.39015

How would I see the new feature? I dont find it in the panels (eg next to the nodes?) or on the map itself.

Thanks

[thomas-hervey]

@tordans, I reproduced your steps and everything should be working. To turn on the layer, look under Quality Assurance at the bottom of the Map Data menu (hotkey F). There is a toggle for KeepRight.

At this point, there are several outstanding tasks including tweaking the post service, tile optimization, more complex translations, custom settings (in the works), and more. Let me know if you still have issues.

[kymckay]

I see there used to be an issue with the error description parsing on error type 411 (http error) due to the URL being collected together with the surrounding tag data (href="URL">URL</a>)). Which would result in:

The URL (<a target="_blank" href="\href=URL>URL</a>)\">{var1}</a>) cannot be opened (HTTP status code 503)

I was looking into this until I realised some of the newer commits resolved the issue, but it got me thinking about the way you're parsing the descriptions in parseErrorDescriptions. Is there any reason you opted not to just extract the details you're looking for by capturing them as regex groups? It would save all the hassle of iterating over the split strings as well as prevent having to add special cases for the descriptions that don't have details split neatly between space characters (like type 220).

The strings you have in errorSchema.json are effectively patterns of a different syntax, in a way you're reinventing the wheel there 😄

Edit: I'd be happy to submit a PR to this branch with changes along those lines if desirable.

[thomas-hervey]

@SilentSpike thanks for your input. Hmm I see what you're saying. As far as that particular error, atm I opted to scratch parsing it and remove the actual URL for safety reasons. No I didn't use string.split() for a particular reason except for simplicity, and regex would be used for identifying IDs. But yes at this point something more consistent like regex groups could help. A PR would be great. I'm only able to spend part time working on this atm.

[tordans]

Thanks @thomas-hervey for helping with #5201 (comment), its working now; issue on my side.

I really like the idea of this PR. Therefore I collected some feedback below. Thanks for this feature!

I collected my first impression

My goal was to collect, not to find solutions :).

a. I experienced the zoom problem from #5169. I needed to zoom out to find an QA-issue which caused the page to become very slow due to a lot of data; afterwards zooming in was tedious.

Update: I found that disabling the OSM-layer helps, but leads to (j) and (k). Its also very manual.

b. I found https://www.keepright.at/report_map.php?schema=100&error=116761974 / http://localhost:8080/#background=Berlin-2018&disable_features=buildings,landuse,boundaries&map=21.00/52.51477/13.38943 but it took me quite a while to understand what the issue is related to, since I did not understand the "way 35557143" (left panel) as the part that is most relevant for this. This element connects the OSM layer with the QA layer and allows me to "select the OSM-element in question". How could that be made clearer?

b. I then tried out to understand the issue and how to fix it. I noticed, that I cannot click the node below the ⚡️-icon. Thats a problem IMO. The only way I could access the pin below the ⚡️was by disabling the QA layer again, that feels like a bad solution. (b) does not help here, since (b) selects the way (for a way-issue), but I wanted to check the node of the way since I assumed an issue there. The node was right below the ⚡️.

The same came up with the second issue that I looked at, an broken URL. The pin is below the ⚡️. In this case (b) helped to select the element in question.

c. For me this issue looks like a false positive. But what do I do with that?

• how can I check the rules that created the issue to better understand if its a false positive?
• should I just say "ignore"? But does "ignore" actualy mean? Will it hide the issue from everyone immediately and forever or just from me?

d. Is there a validation on the comment field? I feel like it should validate to at least 2 word for ignore cases, right? I dont want to test it since I dont know what will happen and the data are live, right?

d2. Keepright actually has thee options "solve, ignore for me, false positive for everyone". Which one are used here / how do they translate to the buttons?

e. I image when I could actually fix the issue, I have the same problem as with #5170, I would like to manually sync my commit message with the get-right-issue. This is the same for HOT Task manager btw, I type my change twice, once in the task manager, once in the commit message. Could the QA layer maybe pre-fill the commit-message with a hash-tag like "#keepright #keepright-issue-123" so there is some link?

f. How do I get from one issue to the next one? Lets say I solved this one, how do I find the next one? The only ways seems to be to zoom out again (see a). But maybe on the post-comment-screen there is a list of the next issues? I will check once I find one that I can solve :).
Update: See (g), since "comment" does not show a post-comment-page ("issue solved, here is the next issue-nearby-list"), I really think discovering the next issue is still a problem.

g. I am looking for a way to share a specific keepright-issue on iD editor. I can share the Keepright-URL, which is provided in iD, but what I was hoping/looking for is: Once I select an KR-issue, the iD URL changes to reflect this issue.

h. I solved my second issue, but now I am very confused about how the comment-field works.

• once I click comment, nothing happens(?)
• however, apparently the comment is saved, because I find it in keepright (the original UI)
• however, it does not behave like a comment field but a "note" field that I can edit at any time? Or is it even one note field for all users, so anyone can edit it?
  This is very unclear to me ATM.

i. To stress (h), the color schema for the ⚡️ is problematic. I did not see the second issue which was a broken URL on the map since it is the same color as buildings, but will be placed on buildings very often. Green is also a hard to see color in nature.

j. When searching for the next ⚡️I find myself disabling the "OSM Data" checkbox because of (a) and (h). However, I also need to disable the QA-Layer because of (b), I scroll around in the panel a lot because OSM is at the top, QA at the bottom.

k. There is a weird behavior that happens with (j): I have OSM data disabled, click on a ⚡️ and inside the ⚡️ on the relation element (see b). Now the UI enables OSM data again (which is very smart, because I need those to see the issue, BUT it does not select the given node. Instead it shows the "choose a tag" UI in the left panel. I found that I need to manually de- and re-activate the OSM data layer, only then does (b) really select the element.

Update: Maybe the last part is not right, I could not solve the issue in every case by re-enabling the layer. Also soft reload of the page did not help. The way of https://www.keepright.at/report_map.php?schema=100&error=102733535 is select (when clicking (b)) in the editor UI (glow) but not visible in the left panel.

[kymckay]

Minor bug I've found while testing. This error (type 70) seems to show up as the wrong error type (I've had 41 and 50) in the iD layer.

[thomas-hervey]

@tordans, thanks for your feedback. Looking into your list of errors now. Hopefully some are handled by @SilentSpike's refactoring ^.

[kymckay]

An update on my previous comment: It should be fixed in my PR (I found that what the wiki lists as error 70 is actually error 74 when inspecting the GeoJson).

[kymckay]

Hmm, this is an interesting case. I have a feeling this won't work for all cases because the pattern will now only match keys with 0 or 1 namespace (which doesn't handle the likes of turn:lanes:backward).

I see that the bit in quotes can be any substring of the key though (I wonder if that also applied to the value). It might be easiest to revert the key group and then just use a .+ in the group within the quotes.

[matkoniecz]

From what I see at https://github.com/keepright/keepright this tools seems to be not maintained anymore

• last commit in February 2017 https://github.com/keepright/keepright/commits/master
• unreviewed PR - Use full dates in rss export keepright/keepright#91
• known false positives waiting for fix (for example Please disable or fix waterway crossing highway warnings, many are in contradiction with wiki and established practice keepright/keepright#60 )

I am not sure whatever exposing iD users to this tool is a good idea.

Maybe Osmose would be a better idea?

[bhousel]

From what I see at https://github.com/keepright/keepright this tools seems to be not maintained anymore
I am not sure whatever exposing iD users to this tool is a good idea.

It's not maintained much, but it does what it's supposed to do, and the server is up. I have a feeling that if iD links to it, people will take more of an interest in maintaining and keeping the server going.

Maybe Osmose would be a better idea?

Yeah, I'd like to do that one too. Also ImproveOSM, and every other Q/A thing that exists! 👍

[matkoniecz]

I have a feeling that if iD links to it, people will take more of an interest in maintaining and keeping the server going.

I have not considered this.

[kymckay]

Example of where this error requires regex:

https://www.keepright.at/report_map.php?schema=86&error=93785774

[bhousel]

Thanks good catch @SilentSpike !

from the source https://github.com/keepright/keepright/blob/dc7a8ada2c87f7cf68d9a1a546cebb53a1f0c281/checks/0070_missing_tags.php#L97-L108

[kymckay]

Oh nice, I hadn't even realised source was available for keepright 😄

[bhousel]

I just merged this! 🎉

Thanks @thomas-hervey for working on this, and I'm looking forward to more integration with other Q/A tools. It will be great to see mappers using this to improve OpenStreetMap!

Thanks also @tordans and @SilentSpike for the detailed feedback on this thread. I've spent the past few weeks improving the user experience and was able to incorporate a lot of your feedback into the latest code. There are also some other good ideas in there that I'll open as follow up issues.

For the curious, you can try it out now in http://preview.ideditor.com/master by enabling the layer on the Map data pane.

[mvl22]

Great news! - thanks :)

Looking forward to seeing this in live.

Just tried this in an edit - makes identifying connectivity problems massively quicker.

[Bibi56]

From what I see at https://github.com/keepright/keepright this tools seems to be not maintained anymore
(...)
I am not sure whatever exposing iD users to this tool is a good idea.

Maybe Osmose would be a better idea?

By selecting Osmose and KeepRight in iD, I got false positive from KeepRight (name and name:XX).
So Osmose is definitively a better option: here KeepRight lower the quality of the database. A QA tool should do the opposite^^.

[jjiglesiasg]

In my experience, both are very useful, why to remove one of them? From: Bibi56 ***@***.*** Sent: Wednesday, May 12, 2021 6:26 PM To: openstreetmap/iD ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [openstreetmap/iD] Keep right QA (#5201) From what I see at https://github.com/keepright/keepright this tools seems to be not maintained anymore (...) I am not sure whatever exposing iD users to this tool is a good idea. Maybe Osmose would be a better idea? By selecting Osmose and KeepRight in iD, I got false positive from KeepRight (name and name:XX <https://github.com/keepright/keepright#106> ). So Osmose is definitively a better option: here KeepRight lower the quality of the database. A QA tool should do the opposite^^. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#5201 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFOKQKRB7FI2RTF32K64JR3TNL6ADANCNFSM4FN2MPFA> . <https://github.com/notifications/beacon/AFOKQKVPWDKA75BBDSW2EPLTNL6ADA5CNFSM4FN2MPFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGIJW3FY.gif> <http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient> Virus-free. <http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient> www.avg.com