feat(graph-rag): publish what one answer costs and what the budget refused - #142
Conversation
…fused The assembler has always measured the token cost of an assembled context and always evicted contributions that did not fit. Nothing published either number, so a deployment could not tell an expensive question from a cheap one, and an answer cut down to fit the budget looked exactly like a whole one. The event gains an optional TokenUsage: the rendered prompt size, the ceiling it was fitted to, the per-channel breakdown, and how many contributions were evicted. Counts only, which is what lets it cross a boundary that refuses queries, prompts and evidence — a number cannot reconstruct the text it measured. It is a projection of the query model's own token record rather than a reference to it, so a retrieval-driven change to that record does not rewrite dashboards. Eviction is counted after merging, not before. Consolidating two copies of one grounding drops nothing the model would have seen, and reporting that as truncation would have put a permanent false positive on the dashboard. Meters accumulate tokens by channel and summarise the prompt size, so headroom is readable before truncation starts. Truncation is counted twice on purpose: how much context was refused, and how many answers were affected — the second is not recoverable from a sum of the first. Tokens are not tagged by organization, which settles the design's "tokens by organization" board against a meter and in favour of a span query; an unbounded tag is paid for by the metrics backend forever rather than by the request that created it. Existing construction sites keep their arity through an overload, so a stage that measures no tokens says so by omission rather than by a zero that would read as a measurement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds validated token-usage events, records dropped grounding contributions during budget fitting, emits context assembly telemetry, and exports token metrics through Micrometer and OpenTelemetry integrations with corresponding test coverage. ChangesContext Token Telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RetrievalService
participant GroundingAssembler
participant EventSink
participant ObservabilityBackend
RetrievalService->>GroundingAssembler: Prepare budgeted grounding
GroundingAssembler-->>RetrievalService: Token counts and dropped contributions
RetrievalService->>EventSink: Emit ASSEMBLE_CONTEXT TokenUsage
EventSink->>ObservabilityBackend: Record token metrics and span attributes
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.java`:
- Around line 107-168: Extend
carriesNoIdentifierThatWouldGrowASeriesPerTenantOrRequest to emit an assembled
context event that exercises the new token and truncation meters, rather than
only a plain RETRIEVE event. Ensure the assertions continue checking every meter
in registry.getMeters() for forbidden org or operation tags, covering
CONTEXT_TOKEN_COUNTER, CONTEXT_PROMPT_TOKENS, CONTEXT_DROPPED_COUNTER, and
CONTEXT_TRUNCATION_COUNTER.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1ea9db0f-ff36-498e-9ea8-34970e83883d
⛔ Files ignored due to path filters (3)
docs/increments/active/2026-07-29-observability-pipeline/plan.mdis excluded by!docs/**docs/specs/domains/secure-graph-rag.mdis excluded by!docs/**docs/tests/domains/secure-graph-rag.mdis excluded by!docs/**
📒 Files selected for processing (11)
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javacomponents/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.javacore/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.java
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: OpenSearch adapter
- GitHub Check: Neo4j adapter
- GitHub Check: PostgreSQL GraphRAG
- GitHub Check: Backend · Java 25
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
**/*: Always read the repository guidance and relevant sections ofARCHITECTURE.md; before changing a domain, read its specification, test-coverage document, and binding decision filenames.
Treat the repository as the engineering system of record; current repository and runtime evidence take precedence over chat or Northstar.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work. Never commit secrets or customer data.
Files:
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.javacomponents/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.javacore/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,gradle,gradle.kts,properties,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Before using unfamiliar Spring Boot 4, Spring Modulith 2, Spring AI 2, or Gradle APIs, consult current official documentation, Context7, and the relevant project verification skill.
Files:
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.javacomponents/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.javacore/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,sql}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
ddl-auto=validateand pair every persisted-model change with a Flyway migration.
Files:
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.javacomponents/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.javacore/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
**/*.{java,gradle,gradle.kts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the testing harness; a terminating clean test is the JVM context gate, and
bootRunis not verification. IDE inspection applies only to edited backend Java.
Files:
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.javacomponents/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.javaintegrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.javaintegrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.javacomponents/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.javacore/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.javacomponents/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java
core/src/main/java/com/orgmemory/core/{authorization,knowledge,permission}/**/*.java
⚙️ CodeRabbit configuration file
core/src/main/java/com/orgmemory/core/{authorization,knowledge,permission}/**/*.java: Treat PostgreSQL ACL evidence as canonical and OpenFGA as the relationship
authorization decision point. Authorization must fail closed. Filtering
must happen before ranking, LIMIT, graph traversal, answer generation,
export, and citation rendering. Flag metadata or timing leak paths.
Files:
core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java
🪛 ast-grep (0.45.0)
integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.java
[warning] 118-118: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(PROMPT_TOKENS, usage.promptTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 119-119: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(SYSTEM_PROMPT_TOKENS, usage.systemPromptTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 120-120: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(QUERY_TOKENS, usage.queryTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 121-121: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(ENTITY_TOKENS, usage.entityTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 122-122: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(RELATION_TOKENS, usage.relationTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 123-123: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(CHUNK_TOKENS, usage.chunkTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 124-124: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(BUDGET_TOKENS, usage.budgetTokens())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 125-125: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(DROPPED_CONTRIBUTIONS, usage.droppedContributions())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
🔇 Additional comments (12)
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/observability/GraphRagEventSink.java (1)
85-122: LGTM!Also applies to: 161-218
components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/SecureContextBudget.java (1)
25-32: LGTM!components/graph-rag-core/src/main/java/com/orgmemory/graphrag/query/LightRagGroundingAssembler.java (1)
81-86: LGTM!Also applies to: 110-124, 126-169, 242-269, 591-635
components/graph-rag-core/src/test/java/com/orgmemory/graphrag/observability/GraphRagEventSinkTests.java (1)
35-62: LGTM!components/graph-rag-testkit/src/test/java/com/orgmemory/graphrag/testkit/LightRagQueryRuntimeConformanceTests.java (1)
295-414: LGTM!core/src/main/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalService.java (2)
18-25: LGTM!Also applies to: 588-632
248-260: 🔒 Security & PrivacyConfirm token/chunk telemetry emitted before final authorization recheck is acceptable.
emitAssembledContextfires right afterconsolidateGrounding, before the post-consolidation scope re-resolve, evidence-closure-size check, andverifyOpenFgarecheck later in this method. The position itself predates this PR (it replaced a genericemitStagecall in the same slot), but this change now attaches a much richer payload (per-channel token counts, chunk counts, dropped contributions) at that pre-recheck point. Since a later scope change/canonical-evidence change can still invalidate this request (seerevocationBetweenRetrievalAndCitationCausesAFullRetryWithoutEgress), please confirm that emitting this expanded metadata before the authorization freshness recheck is intentional and acceptable for this pipeline.As per path instructions, "Filtering must happen before ranking, LIMIT, graph traversal, answer generation, export, and citation rendering. Flag metadata or timing leak paths."
Source: Path instructions
core/src/test/java/com/orgmemory/core/knowledge/GraphRagKnowledgeRetrievalServiceTests.java (1)
432-514: LGTM!Also applies to: 825-850
integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSink.java (1)
25-44: LGTM!Also applies to: 76-106
integrations/graph-rag-observability/src/main/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSink.java (1)
46-61: LGTM!Also applies to: 104-128
integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/MicrometerGraphRagEventSinkTests.java (1)
190-212: LGTM!integrations/graph-rag-observability/src/test/java/com/orgmemory/integrations/graphrag/observability/OpenTelemetryGraphRagEventSinkTests.java (1)
84-184: LGTM!
The guard loops over every registered meter, but only a plain retrieval event had been emitted, so the meters this change introduces were never instantiated and never checked. Verified by mutation: tagging the token counter with organization_id now fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Applied — the finding is right, and it is the one meter test that mattered here: the guard iterates Verified it is not a vacuous assertion by mutation: adding |
Phase 2 of the observability increment. Core has always measured the token cost of an assembled context and always evicted contributions that did not fit the budget. Neither number left the process.
What this closes
Cost is invisible. A deployment cannot tell an expensive question from a cheap one, and cannot see a prompt growing toward its ceiling until it hits it.
Truncation is invisible.
LightRagGroundingAssemblerdrops contributions in two places — the per-channel allocator and the total-budget fitting loop. An answer cut down to fit looked exactly like a whole one, in the logs, in the traces and in the metrics.Shape
GraphRagEventgains an optionalTokenUsage: rendered prompt size, the ceiling it was fitted to, a breakdown across system prompt / query / entity / relation / chunk, and the number of evicted contributions.Counts only. That is what lets it cross a boundary that refuses queries, prompts and evidence — a number cannot reconstruct the text it measured. The span test asserts this by type rather than by name, because a substring guard would reject
query_tokensfor what it is called rather than for what it holds.It is a projection of
ContextTokenUsagerather than a reference to it. That record exists to enforce a retrieval budget and is free to change shape for retrieval reasons; a telemetry contract moving with it would rewrite dashboards for a decision that had nothing to do with them.Existing construction sites keep their arity through an overload, so a stage that measures no tokens says so by omission rather than by a zero that would read as a measurement.
Eviction is counted after merging
Consolidating two copies of one grounding drops nothing the model would have seen — that is deduplication, not budget pressure. Counting it as truncation would have put a permanent false positive on the dashboard. The count is taken after merge and before any budget runs, and a test pins both halves.
I got this wrong first: my initial test asserted that a tight budget over a doubled grounding must evict. It reported zero, correctly, and the test was the thing that was wrong.
Cardinality
Tokens are not tagged by organization. This settles the open question the design left when it asked for a "tokens by organization" board: one series per tenant per channel grows for as long as the product sells, and an unbounded tag is paid for by the metrics backend forever rather than by the request that created it. That board is a span query — the span already carries the organization identifier — or a billing record, which is a product feature rather than a side effect of telemetry.
Truncation gets two meters on purpose. How much context was refused, and how many answers were affected; the second is not recoverable from a sum of the first.
Also
SecureContextBudget.maximumInputTokens()names a rule that was written out three times.Verification
:core:test,:components:graph-rag-core:test,:components:graph-rag-testkit:test,:integrations:graph-rag-observability:test, plus:apps:worker:compileTestJavaand:apps:api:compileTestJava. All green.Not in scope
finish_reason=lengthis output-side truncation and this stage cannot see it:ChatModelPortstreamsFlux<String>and the adapter calls.stream().content(), discarding theChatResponsethat holds the finish reason. It needs a port change and lands with theGENERATEstage.This is also the first commit since #140 that touches application code, so its deployment finally applies the merged-but-unapplied
ORGMEMORY_SERVICE_VERSION/deployment.environment.namefix. I will re-check both on the containers after it deploys.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes