Skip to content

fix(app): allow nlq previews without graph#149

Open
jonathanhaaswriter wants to merge 3 commits intofeat/graph-store-read-foundationfrom
fix/nlq-preview-no-graph
Open

fix(app): allow nlq previews without graph#149
jonathanhaaswriter wants to merge 3 commits intofeat/graph-store-read-foundationfrom
fix/nlq-preview-no-graph

Conversation

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator

Summary

  • allow cerebro.nl_query preview-only requests to translate questions without requiring a readable security graph
  • keep execution mode unchanged so non-preview NLQ requests still fail fast when the graph is unavailable
  • add TDD coverage for the preview-only contract

Validation

  • go test ./internal/app
  • python3 scripts/devex.py run --files internal/app/app_cerebro_tools_nlq.go internal/app/app_cerebro_tools_test.go

Stack

Base branch: feat/graph-store-read-foundation

@jonathanhaaswriter jonathanhaaswriter force-pushed the feat/graph-store-read-foundation branch from b271a42 to f80edf0 Compare March 25, 2026 15:28
@jonathanhaaswriter jonathanhaaswriter force-pushed the fix/nlq-preview-no-graph branch from 89b2b6b to 6be0cac Compare March 25, 2026 15:31
@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator Author

Re-reviewed the current diff and I don't have any additional actionable findings right now.

@jonathanhaaswriter
Copy link
Copy Markdown
Collaborator Author

I think the tenant fail-closed contract regressed here. Once the configured Spanner store advertises tenant read scope, CurrentSecurityGraphStoreForTenant() prefers that path without the HasScopedNodesForTenant() guard the other tenant resolvers use. Because shared nodes are still visible under tenant scope, a tenant-missing request can return shared data instead of ErrStoreUnavailable.

Can we add the same tenant-presence check (or a Spanner equivalent) before returning the scoped store?

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.

1 participant