Skip to content

fix(car): classify access=unknown as restricted instead of fully allowed - #7670

Merged
DennisOSRM merged 1 commit into
Project-OSRM:masterfrom
julcnx:classify-unknown-access-restricted
Jul 31, 2026
Merged

fix(car): classify access=unknown as restricted instead of fully allowed#7670
DennisOSRM merged 1 commit into
Project-OSRM:masterfrom
julcnx:classify-unknown-access-restricted

Conversation

@julcnx

@julcnx julcnx commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Issue

Related to #6762, which raises the broader problem (car profile whitelists all unrecognized access values, including the literal access=unknown, unknown is even named explicitly in that issue's own real-world test data as one of the values whose current handling changes under a stricter fix).

This PR takes a deliberately more conservative position than the patch linked in #6762. That patch treats unrecognized/unknown access values as fully inaccessible. This PR only addresses the literal access=unknown tag value (a real, documented OSM tagging convention meaning "access conditions are unverified, survey needed", see https://wiki.openstreetmap.org/wiki/Tag:access%3Dunknown) and moves it from access_tag_whitelist into restricted_access_tag_list, the same tier private/destination/delivery/customers/permit/residents already occupy.

Reasoning for the softer treatment specifically for unknown: the tag doesn't assert a known restriction the way private/destination/no do, it asserts the opposite, that access status hasn't been determined. Treating it as an outright block asserts something the tag doesn't actually claim. Restricted-tier values aren't excluded from routing by default in this profile today (see way_handlers.lua's access_tag_blacklist check being skipped when forward_restricted/backward_restricted is set), so this change doesn't alter default routability at all, it only gives these ways a correct "restricted" edge class instead of being indistinguishable from access=yes. I think this is worth maintainers' consideration as an alternative (or complement) to the broader fix in #6762, not a replacement for it.

Was this change primarily generated using an AI tool? Yes, Claude (Anthropic, Claude Sonnet 5), based on manual reading of profiles/car.lua, profiles/lib/access.lua, and profiles/lib/way_handlers.lua, and cross-checked against the existing access.feature/classes.feature test suites.

Tasklist

  • self-review code for correctness and following the coding guidelines
  • add tests (new scenario in features/car/classes.feature, modeled on the existing "We tag restricted with a class" test; existing access.feature routability assertions for unknown were checked by hand and remain valid since routability itself doesn't change)
  • update relevant wiki pages
  • review
  • adjust for comments

Requirements / Relations

References #6762 (broader/stricter version of the same underlying gap). Not based on or blocking that PR's own patch.

access=unknown was in access_tag_whitelist, so it was treated
identically to access=yes: fully allowed, no restricted class, no
distinction from a genuinely verified public way.

Move it to restricted_access_tag_list instead, the same tier
private/destination/delivery/customers/permit/residents already use.
Routability is unchanged (restricted-tier values are not excluded by
default, see way_handlers.lua's access_tag_blacklist check being
skipped when forward_restricted/backward_restricted is true), this
only fixes the "restricted" edge classification so API consumers can
see the distinction and, if they choose to, treat it differently.

See Project-OSRM#6762.

Signed-off-by: Julien Etienne <etienne.julien@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the car Lua profile’s access classification so the literal OSM tag value access=unknown is treated as a “restricted” access value (instead of being whitelisted as fully allowed), and adds a cucumber scenario to assert the resulting restricted class tagging in routes.

Changes:

  • Move unknown from access_tag_whitelist to restricted_access_tag_list in profiles/car.lua.
  • Add a cucumber scenario in features/car/classes.feature to verify access=unknown results in the restricted class (including on motorways).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
profiles/car.lua Reclassifies access=unknown from “whitelisted” to “restricted-tier” access handling.
features/car/classes.feature Adds regression coverage asserting access=unknown produces the restricted class in route annotations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread profiles/car.lua
Comment on lines 124 to 128
'customers',
'permit',
'residents',
'unknown',
},
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.15%. Comparing base (da030a7) to head (989bf22).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7670      +/-   ##
==========================================
+ Coverage   90.75%   94.15%   +3.40%     
==========================================
  Files         484      484              
  Lines       37688    37688              
==========================================
+ Hits        34205    35487    +1282     
+ Misses       3483     2201    -1282     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DennisOSRM DennisOSRM left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The reasoning holds up. I checked the OSM wiki page for access=unknown, and it confirms the tag means the access status was not surveyed. It does not mean the way is open. In fact, mappers often use this tag specifically when they suspect a restriction but cannot confirm it. So moving it out of the full-access whitelist and into the restricted tier is the right call.

I also verified the routability claim in way_handlers.lua. Restricted-tier values are not blacklisted by default, so no ways become impassable. This only fixes the API classification.

One small note: this also changes turn penalty behavior, since restricted ways get a high penalty unless they are the start or end of a route. Worth mentioning in the PR description for future reference. Not a blocker.

Nice, well-scoped fix. Approved.

@DennisOSRM
DennisOSRM merged commit e957bf0 into Project-OSRM:master Jul 31, 2026
23 checks passed
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.

3 participants