feat(luminork): Change set review endpoint

[stack72]

Adds a new /review endpoint to the Luminork API that provides a comprehensive, single-call review of all component changes in a change set.

Motivation

Currently, to review all changes in a change set, clients would need to:

1. List all components
2. Fetch individual component diffs for each one
3. Fetch component metadata for subscription resolution
4. Filter out noise (empty defaults, internal fields, etc.)

This requires N+1 API calls and complex client-side logic. For workspaces with 100+ components, this is slow and inefficient.

Solution

New endpoint: GET /v1/w/{workspace_id}/change-sets/{change_set_id}/review

Returns all component diffs with changes in a single API call, with:

• ✅ Pre-filtered attribute diffs (removes noise)
• ✅ Simplified, CLI-friendly format
• ✅ Subscription source resolution included
• ✅ Summary statistics
• ✅ Optional resource code diffs

Implementation Details

Uses Pre-Computed MVs from Frigg

The endpoint leverages existing ComponentDiff materialized views:

• Fetches from Frigg's object store (fast!)
• Returns 202 Accepted if MVs not ready (triggers edda rebuild)
• No expensive on-demand diff calculation

Smart Filtering (Same as Web UI)

Applies the same filtering logic as app/web/src/newhotness/Review.vue:

Excluded diffs:

• Internal fields: /si/type, /si/color
• Identical old/new values (can occur on schema upgrades)
• Empty schema defaults: {}, [], null, "", 0
• Object field placeholders at top-level paths

Diff status recalculation:

• If filtering removes all attribute diffs → Modified becomes None
• Only returns components with meaningful changes

Simplified Response Format

Instead of the complex MV format:

{
  "$source": {
    "component": "01HZZZ...",
    "path": "/domain/region"
  },
  "$value": "us-east-1"
}

Returns CLI-friendly format:

{
  "changeType": "added",
  "newValue": "us-east-1",
  "newSourceType": "subscription",
  "newSourceComponentName": "region-1",
  "newSourcePath": "/domain/region"
}

Design Decisions

Why Not Parallel MV Fetching?

Uses sequential fetching with early filtering instead of parallel. This is fine because:

• MVs are pre-computed and cached (fast to fetch)
• We filter by diff_status early (skip unchanged components)
• Simpler error handling
• Follows existing patterns in the codebase

Why Server-Side Filtering?

• Consistency: Web and CLI show the same filtered view
• Simplicity: CLI doesn't need to reimplement filtering logic
• Single source of truth: Changes to filtering happen in one place
• Better UX: Users see clean, curated diffs

Why Simplified Format?

The raw MV format with $source and $value is:

• Hard to parse
• Verbose
• Requires understanding SI's internal data model

The simplified format is:

• Self-documenting
• Easy to render in CLI
• Clear separation of value vs source

Example Responses

{
  "components": [
    {
      "componentId": "01KF272RATCMXECD5GHD7291B1",
      "componentName": "test",
      "schemaName": "AWS Credential",
      "diffStatus": "Added",
      "attributeDiffs": {
        "/si/name": {
          "changeType": "added",
          "newValue": "test",
          "newSourceType": "value"
        },
        "/secrets/AWS Credential": {
          "changeType": "added",
          "newValue": "fb24d5e22162989a269899c3e3d27654",
          "newSourceType": "value"
        }
      }
    },
    {
      "componentId": "01KF28M419BZXWWXN4EKQPNFKM",
      "componentName": "si-0953",
      "schemaName": "Region",
      "diffStatus": "Added",
      "attributeDiffs": {
        "/si/name": {
          "changeType": "added",
          "newValue": "si-0953",
          "newSourceType": "value"
        },
        "/domain/region": {
          "changeType": "added",
          "newValue": "us-east-1",
          "newSourceType": "value"
        },
        "/secrets/credential": {
          "changeType": "added",
          "newValue": "fb24d5e22162989a269899c3e3d27654",
          "newSourceType": "subscription",
          "newSourceComponentName": "test",
          "newSourceComponentId": "01KF272RATCMXECD5GHD7291B1",
          "newSourcePath": "/secrets/AWS Credential"
        }
      }
    }
  ],
  "summary": {
    "totalComponents": 2,
    "added": 2,
    "modified": 0,
    "removed": 0
  }
}

{
  "components": [
    {
      "componentId": "01KF28M419BZXWWXN4EKQPNFKM",
      "componentName": "si-0953",
      "schemaName": "Region",
      "diffStatus": "Modified",
      "attributeDiffs": {
        "/domain/region": {
          "changeType": "modified",
          "newValue": "us-east-2",
          "oldValue": "us-east-1",
          "newSourceType": "value",
          "oldSourceType": "value"
        }
      }
    }
  ],
  "summary": {
    "totalComponents": 1,
    "added": 0,
    "modified": 1,
    "removed": 0
  }
}

[github-actions]

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

Scanned Files

None

[github-actions]

There are luminork endpoint changes on this branch that do not appear to have associated tests. Please ensure you add or update tests under bin/si-luminork-api-tests/tests/ to satisfy this warning.

[zacharyhamm]

I have a question about this that i'm not quite sure of the answer. Is it possible that by looking at the graph to get the component name could race with the component diff. Should we fetch the components from the materialized view as well? Also a note about not using the cache that is being constructed.

[zacharyhamm]

From the database eh claude? From the graph!

[zacharyhamm]

Should check the all_components_lookup table here first since this component may have already been fetched below

[zacharyhamm]

I don't quite get why there are two component lookup tables. One constructed in the loop and thrown away, and one for the entire loop. Looking at the way the component_lookup is used, I believe it could be removed and just all_component_lookup used?

[zacharyhamm]

Why convert the component id to a string here? (Removing this lookup table would remove that, but you'd have to follow through in the other functions to use component ids instead of their string repr)

[zacharyhamm]

The race condition is probably unlikely and would only occur for diffs that concern the name of a component. Let's try this out for now