Skip to content

Conversation

@ChrisEdwards
Copy link
Collaborator

@ChrisEdwards ChrisEdwards commented Jan 10, 2026

Why

The get_route_coverage tool had severe performance issues caused by an N+1 query pattern:

Root causes:

  1. N+1 Query Pattern: For each route returned, a separate API call fetched route details (observations)
  2. Redundant Data: Full API response included repeated data per route (app info, server lists) that AI agents don't need
  3. Missing Inline Observations: The GET endpoint didn't use expand=observations, forcing the N+1 loop

What Changed

1. API Call Optimization

  • Before: 1 route coverage call + N route detail calls (N+1 pattern)
  • After: 1 route coverage call with observations inlined (single call)

2. Response Size Optimization (~60% reduction)

  • Before: Full Route objects with redundant app, servers, routeDetailsResponse per route
  • After: RouteLight records with only essential fields

3. New Aggregate Statistics

Pre-computed aggregates eliminate client-side computation:

  • totalRoutes - Total route count
  • exercisedCount / discoveredCount - Route status breakdown
  • coveragePercent - Percentage of exercised routes (0.0-100.0, rounded to 2 decimals)
  • totalVulnerabilities / totalCriticalVulnerabilities - Summed across all routes

4. Input Validation Improvement

  • Added validation error for empty array values in sessionMetadataFilters
  • Example: {"branch":[]} now returns clear error instead of silent failure

5. Other changes

  • Deleted unused PaginationHandler.java and related test. (Should have been deleted in prior PRs, but was missed).
  • Updated CLAUDE.md to create better PR descriptions.

How

The implementation follows a layered approach:

Layer 1: SDK - Always Include Observations Inline

File: SDKExtension.java (+20/-52 lines)

The key insight was that TeamServer supports expand=observations to return observations inline, but the SDK wasn't using it:

if (metadata == null) {
  // GET for unfiltered - add expand=observations
  var url = urlBuilder.getRouteCoverageUrl(organizationId, appId) + "&expand=observations";
  is = contrastSDK.makeRequest(HttpMethod.GET, url);
} else {
  // POST for filtered - URL already includes expand=observations
  is = contrastSDK.makeRequestWithBody(HttpMethod.POST, url, gson.toJson(metadata), MediaType.JSON);
}

Also removed the now-unused getRouteDetails() method that powered the N+1 loop.

Layer 2: Data - Add Observations Fields to Route Model

File: Route.java (+6/-2 lines)

Added fields to capture inline observations:

@SerializedName("observations")
private List<Observation> observations;

@SerializedName("total_observations")
private Long totalObservations;

Removed the routeDetailsResponse field that held N+1 query results.

Layer 3: Data - Create Lightweight Response Models

Files: RouteLight.java (56 lines), RouteCoverageResponseLight.java (43 lines)

New immutable Java records that strip redundant data:

public record RouteLight(
    String signature,
    List<String> environments,
    String status,
    String routeHash,
    int vulnerabilities,
    int criticalVulnerabilities,
    long exercised,
    long discovered,
    int serversTotal,
    List<Observation> observations,
    Long totalObservations) {
  
  // Compact constructor ensures observations is never null
  public RouteLight {
    observations = observations != null ? observations : Collections.emptyList();
  }
}

Fields removed from response:

  • app - Application data (already known from request context)
  • servers - Full server list (summary count retained as serversTotal)
  • routeDetailsResponse - Legacy field, now obsolete
  • routeHashString - Duplicate of routeHash

Layer 4: Mapper - Transform to Light Response

File: RouteMapper.java (98 lines)

New @Component mapper that:

  • Transforms RouteRouteLight (strips redundant fields, null-safe)
  • Transforms RouteCoverageResponseRouteCoverageResponseLight
  • Computes aggregate statistics in single-pass loop for efficiency
  • Uses case-insensitive status comparison for robustness
// Single-pass computation for all aggregates
for (var route : routes) {
  if ("EXERCISED".equalsIgnoreCase(route.getStatus())) {
    exercisedCount++;
  } else if ("DISCOVERED".equalsIgnoreCase(route.getStatus())) {
    discoveredCount++;
  }
  totalVulnerabilities += route.getVulnerabilities();
  totalCriticalVulnerabilities += route.getCriticalVulnerabilities();
}

Layer 5: Tool - Remove N+1 Loop

File: GetRouteCoverageTool.java (+11/-14 lines)

The N+1 loop was completely removed:

// DELETED: This entire N+1 loop
// for (Route route : response.getRoutes()) {
//   var routeDetailsResponse = sdkExtension.getRouteDetails(orgId, params.appId(), route.getRouteHash());
//   route.setRouteDetailsResponse(routeDetailsResponse);
// }

// NEW: Transform to light response (observations already inline)
return routeMapper.toResponseLight(response);

Layer 6: Validation - Improve Error Messages

File: MetadataJsonFilterSpec.java (+6/-1 lines)

Added explicit validation for empty array values:

if (list.isEmpty()) {
  invalidEntries.add(
      String.format("'%s' (empty array - must have at least one value)", fieldName));
  return null;
}

Step-by-Step Walkthrough

Phase 1: Enable Inline Observations (Commits 1-5)

Commit 1 (b385d5c): Added observations and totalObservations fields to Route.java so Gson can deserialize inline observations.

Commit 2 (c42c7d6): First attempt - switched to always use POST endpoint with expand=observations. This worked for filtered requests.

Commit 3 (8bd993d): Removed the N+1 loop from GetRouteCoverageTool since observations are now inline.

Commit 4 (7a536d5): Tried using empty request object {} for unfiltered requests, but POST requires actual filter criteria.

Commit 5 (dd6ab8f): Final solution - use GET with expand=observations for unfiltered requests, POST for filtered. Both include observations inline.

Phase 2: Lightweight Response Models (Commits 6-9)

Commit 6 (6f3af05): Added RouteLight.java record with essential fields only.

Commit 7 (80162c4): Added RouteCoverageResponseLight.java record with aggregate statistics fields.

Commit 8 (19718d2): Added RouteMapper.java to transform full responses to light versions with computed aggregates.

Commit 9 (e3514c8): Integrated RouteMapper into GetRouteCoverageTool, changed return type to RouteCoverageResponseLight.

Phase 3: Code Review Refinements (Commits 10-11)

Commit 10 (e442af9): Applied code review feedback:

  • RouteLight: Added compact constructor ensuring observations never null
  • RouteMapper: Case-insensitive status comparison, explicit DISCOVERED count, coverage percent rounded to 2 decimals
  • MetadataJsonFilterSpec: Added validation error for empty array values

Commit 11 (f838feb): Final optimizations:

  • Changed aggregate computation from 4 stream operations to single-pass loop
  • Removed unused routeDetailsResponse field from Route
  • Deleted getRouteDetails() method from SDKExtension
  • Deleted RouteDetailsResponse.java class entirely

Testing

New Test Files

File Tests Coverage
RouteTest.java 2 JSON deserialization of observations
SDKExtensionTest.java 2 GET/POST endpoint selection with expand param
RouteLightTest.java 3 Record immutability, null handling, field values
RouteCoverageResponseLightTest.java 4 Aggregate computation edge cases
RouteMapperTest.java 8 Transformation logic, null safety, status counting
MetadataJsonFilterSpecTest.java +1 Empty array validation error

Updated Test Files

File Changes
GetRouteCoverageToolTest.java Updated to use record accessors, added aggregate tests
GetRouteCoverageToolIT.java Updated to work with RouteLight records

Results

✓ Format check passed (make check)
✓ Unit tests passed (make test)
  Tests run: 614, Failures: 0, Errors: 0, Skipped: 0
✓ Integration tests passed (make verify)

Files Changed Summary

Category Files Lines
New data models 2 +99
New mapper 1 +98
SDK changes 1 +20/-52
Tool changes 1 +11/-14
Validation 1 +6/-1
Deleted 1 -29
New tests 5 +513
Updated tests 3 +135/-108
Total 16 files +888/-206

@ChrisEdwards ChrisEdwards marked this pull request as ready for review January 10, 2026 05:52
POST /route/filter requires sessionID or metadata filter - cannot send empty body.
Revert to GET/POST logic but add expand=observations to GET URL to include
observations inline and eliminate N+1 queries.
Lightweight response wrapper for route coverage that holds:
- API status (success, messages)
- Aggregate statistics (totalRoutes, exercisedCount, discoveredCount, coveragePercent)
- Vulnerability totals (totalVulnerabilities, totalCriticalVulnerabilities)
- List of RouteLight objects

Part of route coverage N+1 elimination effort. Task 2 of 5.
Implements mapper component that:
- Transforms Route -> RouteLight (removes redundant app, servers,
  routeDetailsResponse, routeHashString fields)
- Transforms RouteCoverageResponse -> RouteCoverageResponseLight
  (computes aggregate statistics: coverage percent, exercised/discovered
  counts, vulnerability totals)
- Handles null safety for lists (returns empty list instead of null)

Part of route coverage N+1 elimination effort.
…nses

Update GetRouteCoverageTool to return RouteCoverageResponseLight instead
of the full RouteCoverageResponse. This reduces payload size for AI agents
by eliminating redundant fields (app, servers, routeHashString, routeDetailsResponse)
and adds aggregate statistics (exercisedCount, discoveredCount, coveragePercent,
totalVulnerabilities, totalCriticalVulnerabilities).

- Add RouteMapper dependency via constructor injection
- Change return type from RouteCoverageResponse to RouteCoverageResponseLight
- Update unit tests to use record accessors instead of getters
- Add test for aggregate statistics computation
- Update integration tests to work with RouteLight records
- RouteLight: add compact constructor to ensure observations is never null
- RouteMapper: use case-insensitive status comparison for robustness
- RouteMapper: count DISCOVERED explicitly instead of subtracting from total
- RouteMapper: round coveragePercent to 2 decimal places
- MetadataJsonFilterSpec: add validation error for empty array values
…putation

- Use single-pass loop in RouteMapper for aggregate statistics instead
  of 4 separate stream operations (minor performance improvement)
- Remove unused routeDetailsResponse field from Route class
- Remove getRouteDetails() method from SDKExtension (N+1 pattern eliminated)
- Delete RouteDetailsResponse class (no longer needed)
- Update tests to remove references to deleted method
@ChrisEdwards ChrisEdwards force-pushed the AIML-365-route-coverage-n-plus-1 branch from 6006a0f to f838feb Compare January 11, 2026 03:25
@ChrisEdwards ChrisEdwards changed the base branch from AIML-362-status-filter-fix to main January 11, 2026 03:25
- Delete PaginationHandler.java and PaginationHandlerTest.java (unused)
- Add .tldr/ and .tldrignore to gitignore (TLDR tool artifacts)
- Update CLAUDE.md with PR description guidance
@ChrisEdwards ChrisEdwards merged commit 2798f6d into main Jan 12, 2026
4 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.

4 participants