diff --git a/DEVELOPER.md b/DEVELOPER.md index 17c480b..be814ba 100644 --- a/DEVELOPER.md +++ b/DEVELOPER.md @@ -303,21 +303,17 @@ These are internal app tags, not OSM tags. The underscore prefix makes this expl - **Surgical detection**: Only splits on the actual 50k node limit, not timeouts or other network issues. -**Query optimization & deduplication:** -- **Pre-fetch limit**: 4x user's display limit (e.g., 1000 nodes for 250 display limit) -- **Profile deduplication**: Automatically removes redundant profiles from queries using subsumption analysis -- **User-initiated detection**: Only reports loading status for user-facing operations -- **Background operations**: Pre-fetch runs silently, doesn't trigger loading states - -**Profile subsumption optimization (v2.1.1+):** -To reduce Overpass query complexity, profiles are deduplicated before query generation: +**Query optimization & deduplication (in `OverpassService._buildQuery`):** +Profiles are deduplicated before query generation via subsumption analysis: - **Subsumption rule**: Profile A subsumes profile B if all of A's non-empty tags exist in B with identical values - **Example**: `Generic ALPR` (tags: `man_made=surveillance, surveillance:type=ALPR`) subsumes `Flock` (same tags + `manufacturer=Flock Safety`) - **Result**: Default profile set reduces from ~11 to ~2 query clauses (Generic ALPR + Generic Gunshot) -- **UI unchanged**: All enabled profiles still used for post-query filtering and display matching +- **UI unchanged**: All enabled profiles still used for post-query filtering and display matching (see `NodeProviderWithCache._matchesAnyProfile`), independent of which clauses were sent to Overpass +- **Safety fallback**: if dedup would remove every profile (e.g. all profiles have only empty tags), the original full list is used instead of sending an empty query **Why this approach:** -Dense urban areas (SF, NYC) with many profiles enabled can easily exceed both 50k node limits and 25s timeouts. Profile deduplication reduces query complexity by ~80% for default setups, while automatic splitting handles remaining edge cases. Surgical error detection avoids unnecessary API load from network issues. +Dense urban areas (SF, NYC) with many profiles enabled can easily exceed both 50k node limits and 45s timeouts. Profile deduplication reduces query complexity by ~80% for default setups. Surgical error detection avoids unnecessary API load from network issues. + ### 6. Uploader Service Architecture (Refactored v1.5.3) diff --git a/lib/dev_config.dart b/lib/dev_config.dart index 9fe96d6..d5127b0 100644 --- a/lib/dev_config.dart +++ b/lib/dev_config.dart @@ -162,11 +162,11 @@ const int kAbsoluteMaxTileCount = 50000; const int kAbsoluteMaxZoom = 23; // Node icon configuration -const double kNodeIconDiameter = 18.0; -const double _kNodeRingThicknessBase = 2.5; -const double kNodeDotOpacity = 0.3; // Opacity for the grey dot interior +const double kNodeIconDiameter = 17.0; +const double _kNodeRingThicknessBase = 2.6; +const double kNodeDotOpacity = 0.28; // Opacity for the grey dot interior const Color kNodeRingColorReal = Color(0xFF3036F0); // Real nodes from OSM - blue -const Color kNodeRingColorMock = Color(0xD0FFFFFF); // Add node mock point - white +const Color kNodeRingColorMock = Color(0xE0AAAAAA); // Add node mock point - white const Color kNodeRingColorPending = Color(0xD09C27B0); // Submitted/pending nodes - purple const Color kNodeRingColorEditing = Color(0xD0FF9800); // Node being edited - orange const Color kNodeRingColorPendingEdit = Color(0xD0757575); // Original node with pending edit - grey diff --git a/lib/services/overpass_service.dart b/lib/services/overpass_service.dart index bb47a86..b354fd1 100644 --- a/lib/services/overpass_service.dart +++ b/lib/services/overpass_service.dart @@ -126,9 +126,24 @@ class OverpassService { return ErrorDisposition.retry; } - /// Build Overpass QL query for given bounds and profiles + /// Build Overpass QL query for given bounds and profiles. + /// + /// Profiles are deduplicated first: if one profile's non-empty tags are a + /// subset of another's (with identical values), the broader profile's query + /// clause already returns every node the narrower one would, so the + /// narrower clause is redundant and can be dropped. This directly reduces + /// query size/complexity without changing the result set — client-side + /// filtering (matching returned nodes against the full profile list) still + /// happens independently, so nothing is lost. String _buildQuery(LatLngBounds bounds, List profiles) { - final nodeClauses = profiles.map((profile) { + final profilesToQuery = _deduplicateProfilesForQuery(profiles); + + if (profilesToQuery.length < profiles.length) { + debugPrint( + '[OverpassService] Deduplicated ${profiles.length} profiles to ${profilesToQuery.length} for query efficiency'); + } + + final nodeClauses = profilesToQuery.map((profile) { // Convert profile tags to Overpass filter format, excluding empty values final tagFilters = profile.tags.entries .where((entry) => entry.value.trim().isNotEmpty) @@ -149,6 +164,74 @@ out ids; '''; } + /// Deduplicate profiles for Overpass queries by removing profiles that are + /// subsumed by others. A profile A subsumes profile B if all of A's + /// non-empty tags exist in B with identical values — meaning every node + /// matching B's filter also matches A's filter, so B's clause is redundant. + List _deduplicateProfilesForQuery(List profiles) { + if (profiles.length <= 1) return profiles; + + final result = []; + + for (final candidate in profiles) { + // Skip profiles that only have empty tags - they would match + // everything and shouldn't be treated as subsuming or subsumable. + final candidateNonEmptyTags = candidate.tags.entries + .where((entry) => entry.value.trim().isNotEmpty) + .toList(); + + if (candidateNonEmptyTags.isEmpty) continue; + + // Check if any existing profile in our result subsumes this candidate + bool isSubsumed = false; + for (final existing in result) { + if (_profileSubsumes(existing, candidate)) { + isSubsumed = true; + break; + } + } + + if (!isSubsumed) { + // This candidate is not subsumed, so add it. But first, remove any + // existing profiles that this candidate subsumes (candidate is + // broader, so those clauses are now redundant). + result.removeWhere((existing) => _profileSubsumes(candidate, existing)); + result.add(candidate); + } + } + + // Safety check: if dedup removed everything (e.g. all-empty-tag + // profiles), fall back to the original list rather than sending an + // empty query. + return result.isNotEmpty ? result : profiles; + } + + /// Check if [broaderProfile] subsumes [specificProfile]: true if all + /// non-empty tags in [broaderProfile] exist in [specificProfile] with + /// identical values. + bool _profileSubsumes(NodeProfile broaderProfile, NodeProfile specificProfile) { + final broaderTags = Map.fromEntries( + broaderProfile.tags.entries.where((entry) => entry.value.trim().isNotEmpty), + ); + final specificTags = Map.fromEntries( + specificProfile.tags.entries.where((entry) => entry.value.trim().isNotEmpty), + ); + + // If broader has no non-empty tags, it doesn't subsume anything (it + // would match everything, which isn't a meaningful "broader filter"). + if (broaderTags.isEmpty) return false; + + // If broader has more non-empty tags than specific, it can't subsume it. + if (broaderTags.length > specificTags.length) return false; + + for (final entry in broaderTags.entries) { + if (specificTags[entry.key] != entry.value) return false; + } + + return true; + } + + /// Parse Overpass JSON response into OsmNode objects List _parseResponse(String responseBody) { final data = jsonDecode(responseBody) as Map; diff --git a/test/services/overpass_service_test.dart b/test/services/overpass_service_test.dart index 91a19da..6c52566 100644 --- a/test/services/overpass_service_test.dart +++ b/test/services/overpass_service_test.dart @@ -99,6 +99,129 @@ void main() { }); }); + group('profile deduplication (subsumption)', () { + test('a specific profile subsumed by a generic one is dropped', () async { + final generic = NodeProfile( + id: 'generic-alpr', + name: 'Generic ALPR', + tags: const { + 'man_made': 'surveillance', + 'surveillance:type': 'ALPR', + }, + ); + final specific = NodeProfile( + id: 'flock', + name: 'Flock', + tags: const { + 'man_made': 'surveillance', + 'surveillance': 'public', + 'surveillance:type': 'ALPR', + 'surveillance:zone': 'traffic', + 'camera:type': 'fixed', + 'manufacturer': 'Flock Safety', + 'manufacturer:wikidata': 'Q108485435', + }, + ); + + stubOverpassResponse([]); + + await service.fetchNodes(bounds: bounds, profiles: [generic, specific]); + + final captured = verify( + () => mockClient.post(any(), body: captureAny(named: 'body')), + ).captured; + final query = (captured.last as Map)['data']!; + + // Only the generic clause should remain. + expect(query, contains('["man_made"="surveillance"]["surveillance:type"="ALPR"]')); + expect(query, isNot(contains('manufacturer'))); + expect(query, isNot(contains('Flock Safety'))); + + // Only one node clause total. + final nodeClauseCount = RegExp(r'node\[').allMatches(query).length; + expect(nodeClauseCount, equals(1)); + }); + + + test('unrelated profiles are all kept', () async { + final alprGeneric = NodeProfile( + id: 'generic-alpr', + name: 'Generic ALPR', + tags: const { + 'man_made': 'surveillance', + 'surveillance:type': 'ALPR', + }, + ); + final gunshotGeneric = NodeProfile( + id: 'generic-gunshot', + name: 'Generic Gunshot Detector', + tags: const { + 'man_made': 'surveillance', + 'surveillance:type': 'gunshot_detector', + }, + ); + + stubOverpassResponse([]); + + await service.fetchNodes(bounds: bounds, profiles: [alprGeneric, gunshotGeneric]); + + final captured = verify( + () => mockClient.post(any(), body: captureAny(named: 'body')), + ).captured; + final query = (captured.last as Map)['data']!; + + expect(query, contains('["surveillance:type"="ALPR"]')); + expect(query, contains('["surveillance:type"="gunshot_detector"]')); + }); + + test('full default profile set reduces to the two generic clauses', () async { + final profiles = NodeProfile.getDefaults(); + + stubOverpassResponse([]); + + await service.fetchNodes(bounds: bounds, profiles: profiles); + + final captured = verify( + () => mockClient.post(any(), body: captureAny(named: 'body')), + ).captured; + final query = (captured.last as Map)['data']!; + + final nodeClauseCount = RegExp(r'node\[').allMatches(query).length; + expect(nodeClauseCount, equals(2)); + + expect(query, contains('["man_made"="surveillance"]["surveillance:type"="ALPR"]')); + expect(query, contains('["man_made"="surveillance"]["surveillance:type"="gunshot_detector"]')); + // Brand-specific tags should not appear anywhere in the reduced query. + expect(query, isNot(contains('manufacturer'))); + expect(query, isNot(contains('ShotSpotter'))); + }); + + test('profiles with only empty tags do not subsume or break the query', () async { + final emptyTagsProfile = NodeProfile( + id: 'empty', + name: 'Empty', + tags: const {'camera:mount': ''}, + ); + final normalProfile = NodeProfile( + id: 'normal', + name: 'Normal', + tags: const {'man_made': 'surveillance'}, + ); + + stubOverpassResponse([]); + + await service.fetchNodes(bounds: bounds, profiles: [emptyTagsProfile, normalProfile]); + + final captured = verify( + () => mockClient.post(any(), body: captureAny(named: 'body')), + ).captured; + final query = (captured.last as Map)['data']!; + + expect(query, contains('["man_made"="surveillance"]')); + }); + }); + + group('response parsing — constraint detection', () { test('nodes referenced by a way are constrained', () async { stubOverpassResponse([