perf(retrieval): fair snapshot admission, pool right-sizing, and TTFT attribution - #292
Conversation
Consolidates the two-model architecture debate into decision 0020 and opens the Phase 1 increment: fair 4-permit retrieval admission, API pool 8/8, topK reverted to the upstream default, and assistant-layer TTFT attribution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds payload-free assistant stage telemetry across the core and API modules. It instruments chat-memory loads and assistant turn stages through Micrometer and OpenTelemetry. It also adds fair retrieval admission control, concurrent snapshot execution, new runtime defaults, and related tests. ChangesAssistant observability
Retrieval control
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AssistantService
participant ObservedChatMemory
participant AssistantStageEventSink
participant OpenTelemetryAssistantStageEventSink
participant MicrometerAssistantStageEventSink
Client->>AssistantService: start assistant turn
AssistantService->>ObservedChatMemory: load conversation history
ObservedChatMemory->>AssistantStageEventSink: emit history-load event
AssistantService->>AssistantStageEventSink: emit turn-stage events
AssistantStageEventSink->>OpenTelemetryAssistantStageEventSink: export span
AssistantStageEventSink->>MicrometerAssistantStageEventSink: record timer
AssistantService-->>Client: stream generated tokens
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
TegamiThis repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under Create a changelog → · Changelog format Release preview
Changelogs in this PR
Run Managed by Tegami. |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@core/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.java`:
- Around line 119-121: Remove the convenience constructors in
DefaultGraphRagKnowledgeRetrievalService that instantiate
RetrievalAdmissionControl, requiring callers to provide the shared instance
explicitly. Update the remaining direct test construction to pass the existing
admission-control instance and preserve the configuration/helper call sites that
already inject one.
- Around line 592-598: Update queryPublishedSpace so the existing
SnapshotQueryResult.duration() measures only engine.executePrepared query
execution, starting its timer after admission.execute grants execution. Do not
include permit acquisition time in that duration; if admission latency must be
exposed, add it as a separate SnapshotQueryResult field rather than changing the
existing duration semantics.
- Around line 540-566: Clarify the concurrency contract around
maximumConcurrentSpaces: the fan-out in the retrieval task now starts all
requests, while shared admission is enforced separately. Decide whether
retrieval-admission-permits is the sole concurrency boundary; if so, deprecate
or remove the unused maximumConcurrentSpaces configuration and key, otherwise
implement an independent limit and document its scope. Update the related
validation and binding so the configuration behavior matches this decision.
In
`@core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java`:
- Around line 23-30: Update RetrievalAdmissionControl.execute(Callable) and the
AssistantService.search() flow to run retrieval through a Future and await it
with the turn timeout via bounded Future.get(timeout, TimeUnit). On timeout or
turn cancellation, cancel the Future, handle InterruptedException and
CancellationException, and ensure abandoned work releases any acquired admission
permit instead of blocking indefinitely.
🪄 Autofix
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: 735f0f5c-650f-4f75-9f7b-0df608f2e0c5
⛔ Files ignored due to path filters (4)
docs/decisions/0020-authorized-multi-snapshot-query-plane.mdis excluded by!docs/**docs/increments/active/2026-08-05-retrieval-admission-phase1/design.mdis excluded by!docs/**docs/increments/active/2026-08-05-retrieval-admission-phase1/plan.mdis excluded by!docs/**docs/roadmap.mdis excluded by!docs/**
📒 Files selected for processing (22)
apps/api/build.gradle.ktsapps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.javaapps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.javaapps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.javaapps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.javaapps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.javaapps/api/src/main/resources/application-prod.ymlapps/api/src/main/resources/application.ymlapps/api/src/test/java/com/orgmemory/api/MetricsDistributionTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/GraphRagQueryRuntimePropertiesTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSinkTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/ObservedChatMemoryTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSinkTests.javacore/src/main/java/com/orgmemory/core/assistant/AssistantService.javacore/src/main/java/com/orgmemory/core/assistant/observability/AssistantStageEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalConfiguration.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.javacore/src/test/java/com/orgmemory/core/assistant/AssistantTurnObservationTests.javacore/src/test/java/com/orgmemory/core/assistant/observability/AssistantTurnEventTests.javacore/src/test/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalServiceTests.java
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Web · Node 24
- GitHub Check: Backend · Java 25
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Treat the repository and runtime evidence as the engineering system of record; do not treat chat or Northstar as authoritative.
Before changing a domain, read its specification, test-coverage document, and applicable decision filenames.
Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, parity scope, or deployment require an independent architecture challenge and documented alternatives before implementation.
Do not use completed increment documents as the source for current behavior; use them only for history or archaeology.
Before using unfamiliar Spring Boot, Spring Modulith, Spring AI, Gradle, React, Vite, Tailwind, TypeScript, Next.js, or Fumadocs APIs, consult current official documentation, Context7, and the relevant verification skill.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work; never commit secrets or customer data.
Keepddl-auto=validateand pair every persisted-model change with a Flyway migration.
Use the testing harness; a terminating clean test is the JVM context gate, andbootRunis not verification.
Files:
apps/api/src/test/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSinkTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.javaapps/api/src/test/java/com/orgmemory/api/assistant/ObservedChatMemoryTests.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.javaapps/api/src/main/resources/application-prod.ymlapps/api/build.gradle.ktsapps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalConfiguration.javacore/src/main/java/com/orgmemory/core/assistant/observability/AssistantStageEventSink.javaapps/api/src/test/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSinkTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.javaapps/api/src/main/resources/application.ymlcore/src/test/java/com/orgmemory/core/assistant/observability/AssistantTurnEventTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/GraphRagQueryRuntimePropertiesTests.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.javaapps/api/src/test/java/com/orgmemory/api/MetricsDistributionTests.javacore/src/test/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalServiceTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.javaapps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.javacore/src/test/java/com/orgmemory/core/assistant/AssistantTurnObservationTests.javacore/src/main/java/com/orgmemory/core/assistant/AssistantService.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Apply IDE inspection only to edited backend Java files.
Files:
apps/api/src/test/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSinkTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.javaapps/api/src/test/java/com/orgmemory/api/assistant/ObservedChatMemoryTests.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.javaapps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalConfiguration.javacore/src/main/java/com/orgmemory/core/assistant/observability/AssistantStageEventSink.javaapps/api/src/test/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSinkTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.javacore/src/test/java/com/orgmemory/core/assistant/observability/AssistantTurnEventTests.javaapps/api/src/test/java/com/orgmemory/api/assistant/GraphRagQueryRuntimePropertiesTests.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.javaapps/api/src/test/java/com/orgmemory/api/MetricsDistributionTests.javacore/src/test/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalServiceTests.javaapps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.javaapps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.javacore/src/test/java/com/orgmemory/core/assistant/AssistantTurnObservationTests.javacore/src/main/java/com/orgmemory/core/assistant/AssistantService.java
apps/api/src/main/java/**/*.java
⚙️ CodeRabbit configuration file
apps/api/src/main/java/**/*.java: Enforce the browser-BFF and resource-server boundaries. Authentication
must resolve an active internal actor through the explicit issuer and
subject binding. Reject identity, tenant, roles, or permissions supplied
by request payloads, JWT email, or untrusted JWT role claims.
Files:
apps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.javaapps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.javaapps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.javaapps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.javaapps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.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/retrieval/RetrievalAdmissionControl.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalConfiguration.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.javacore/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.java
🧠 Learnings (1)
📚 Learning: 2026-07-26T05:46:47.443Z
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 61
File: apps/mcp/src/main/java/com/orgmemory/mcp/McpSecurityConfiguration.java:50-52
Timestamp: 2026-07-26T05:46:47.443Z
Learning: In OrgMemory, treat the `apps/mcp` and `apps/api` as independent protocol adapter modules. When adjusting OAuth/wire-level scopes, do not introduce a shared Java constant or create a code dependency from `apps/mcp` to `apps/api` solely to deduplicate scope values. Instead, keep OAuth/scope constants adapter-local (e.g., in the relevant adapter/security configuration classes) and ensure cross-adapter consistency via automated realm/OAuth/authorization tests, rather than via shared wiring-level constants or cross-module references.
Applied to files:
apps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.javaapps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.javaapps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.javaapps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.javaapps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.java
🪛 ast-grep (0.45.0)
apps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.java
[warning] 52-52: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(ENGINE, value(event.engine()))
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 53-53: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(STAGE, value(event.stage()))
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 54-54: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(OUTCOME, value(event.outcome()))
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 55-57: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(
DURATION_NANOS,
event.duration().toNanos())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
[warning] 59-59: Avoid using untrusted input as a setAttribute() name (trust boundary violation)
Context: span.setAttribute(FAILURE_CODE, event.failureCode())
Note: [CWE-501] Trust Boundary Violation.
(trust-boundaries-java)
🔇 Additional comments (22)
core/src/main/java/com/orgmemory/core/assistant/observability/AssistantStageEventSink.java (1)
1-86: LGTM!core/src/main/java/com/orgmemory/core/assistant/AssistantService.java (1)
7-8: LGTM!Also applies to: 18-19, 36-36, 49-67, 116-156, 178-191, 215-230
core/src/test/java/com/orgmemory/core/assistant/AssistantTurnObservationTests.java (1)
11-20: LGTM!Also applies to: 38-38, 60-61, 178-211
core/src/test/java/com/orgmemory/core/assistant/observability/AssistantTurnEventTests.java (1)
22-34: LGTM!apps/api/src/test/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSinkTests.java (1)
1-41: LGTM!apps/api/src/test/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSinkTests.java (1)
1-87: LGTM!apps/api/src/test/java/com/orgmemory/api/MetricsDistributionTests.java (1)
48-48: LGTM!Also applies to: 67-67
apps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.java (1)
7-7: LGTM!Also applies to: 19-21, 41-52, 75-94
apps/api/src/main/java/com/orgmemory/api/assistant/ObservedChatMemory.java (1)
1-71: LGTM!apps/api/src/main/java/com/orgmemory/api/assistant/OpenTelemetryAssistantStageEventSink.java (1)
1-79: LGTM!apps/api/src/main/java/com/orgmemory/api/assistant/MicrometerAssistantStageEventSink.java (1)
1-37: LGTM!apps/api/build.gradle.kts (1)
45-45: LGTM!apps/api/src/test/java/com/orgmemory/api/assistant/ObservedChatMemoryTests.java (1)
1-79: LGTM!core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.java (1)
14-14: LGTM!Also applies to: 29-29, 74-75
apps/api/src/main/java/com/orgmemory/api/assistant/GraphRagQueryRuntimeProperties.java (1)
15-15: LGTM!Also applies to: 49-53, 106-106
core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java (1)
15-21: LGTM!Also applies to: 32-38
core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalConfiguration.java (1)
18-24: LGTM!Also applies to: 38-38, 55-56
core/src/main/java/com/orgmemory/core/knowledge/retrieval/DefaultGraphRagKnowledgeRetrievalService.java (1)
46-46: LGTM!Also applies to: 91-91, 184-184, 608-611
core/src/test/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalServiceTests.java (1)
6-6: LGTM!Also applies to: 32-40, 85-88, 178-216, 218-269, 271-327, 1062-1062, 1085-1105, 1144-1184
apps/api/src/test/java/com/orgmemory/api/assistant/GraphRagQueryRuntimePropertiesTests.java (1)
26-40: LGTM!Also applies to: 80-107
apps/api/src/main/resources/application.yml (1)
153-154: LGTM!apps/api/src/main/resources/application-prod.yml (1)
10-11: 🩺 Stability & AvailabilityNo change needed.
The assistant retrieval path completes
retrieval.search(...)before streaming, andretrieval-admission-permits: 4 + pool-size 8 = 12, leaving connections for the active request after admission.> Likely an incorrect or invalid review comment.
| <T> T execute(Callable<T> query) throws Exception { | ||
| permits.acquire(); | ||
| try { | ||
| return query.call(); | ||
| } finally { | ||
| permits.release(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace how the assistant turn timeout terminates in-flight retrieval work.
set -euo pipefail
# Locate the turn-timeout consumer and check for interrupt-capable cancellation.
rg -n -C 6 'turn-timeout|turnTimeout' --type=java
# Inspect the assistant orchestration for timeout enforcement primitives.
fd -e java --iglob '*Assistant*' | while IFS= read -r file; do
rg -n -C 4 'orTimeout|completeOnTimeout|awaitTermination|cancel\(|Future|shutdownNow' "$file" || true
done
# Check whether any caller bounds the retrieval search call itself.
rg -n -C 5 '\.search\(' --type=java -g '!**/test/**'Repository: kl3inIT/OrgMemory
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Java files matching retrieval/admission/assistant names:\n'
fd -e java -i 'RetrievalAdmissionControl|assistant|Assistant|turn-timeout|turnTimeout|publishedSpaces' . | sed -n '1,120p'
printf '\nSearch timeout/cancel/acquire/take usages:\n'
rg -n -C 4 'acquir|turn-timeout|turnTimeout|orTimeout|completeOnTimeout|awaitTermination|cancel\(|shutdownNow|completed\.take|RetrievalAdmissionControl' --type=java || true
printf '\nGit status/stat:\n'
git status --short
git diff --stat HEAD || trueRepository: kl3inIT/OrgMemory
Length of output: 6198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files mentioning ORGMEMORY_ASSISTANT_TURN_TIMEOUT / assistant timeout / turn timeout:\n'
rg -n -i 'ORGMEMORY_ASSISTANT_TURN_TIMEOUT|ASSISTANT_TURN_TIMEOUT|Assistant.*Timeout|turn.*timeout|timeout.*turn' . || true
printf '\nRetrievalAdmissionControl.java:\n'
cat -n core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java
printf '\nAssistantConfiguration.java:\n'
cat -n apps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.java
printf '\nAssistantProperties.java:\n'
cat -n apps/api/src/main/java/com/orgmemory/api/assistant/AssistantProperties.java
printf '\nAssistantController.java relevant excerpts:\n'
wc -l apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
sed -n '1,260p' apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
printf '\nAssistantService.java if present:\n'
if [ -f core/src/main/java/com/orgmemory/core/assistant/AssistantService.java ]; then
rg -n -C 5 'Future|Executor|submit|CompletableFuture|timeout|queryPublishedSpaces|retrieval|search' core/src/main/java/com/orgmemory/core/assistant/AssistantService.java || true
fiRepository: kl3inIT/OrgMemory
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Focused search for assistant turn timeout implementation:\n'
rg -n -C 4 'withTimeout|scheduledExecutorService|setTimeout|ORGMEMORY_ASSISTANT_TURN_TIMEOUT|turnTimeout|completed\.take|CompletionQueue|RetrievalAdmissionControl|permit' --type=java . | sed -n '1,240p'
printf '\nFocused listing for timeout-related files:\n'
fd -e java -i 'Timeout|turn|completion|queue|retrieval|Assistant' . | sed -n '1,200p'
printf '\nAssistant controller focused excerpts:\n'
rg -n -C 8 'get|Future|CompletableFuture|Thread|interrupt|timeout' apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java || trueRepository: kl3inIT/OrgMemory
Length of output: 41342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'AssistantController.java outline and relevant excerpt:\n'
wc -l apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java
sed -n '1,170p' apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController|cat -n
printf '\nRetrievalAdmissionControl exact file:\n'
cat -n core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java
printf '\nAssistantService.java outline and relevant terms:\n'
wc -l core/src/main/java/com/orgmemory/core/assistant/AssistantService.java
rg -n -C 8 'Future|Executor|submit|CompletableFuture|turnTimeout|timeout|search|queryPublishedSpaces|RetrievalAdmissionControl|cancel|interrupt' core/src/main/java/com/orgmemory/core/assistant/AssistantService.java || trueRepository: kl3inIT/OrgMemory
Length of output: 404
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'AssistantController.java outline:\n'
ast-grep outline apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java --view compact || true
printf '\nAssistantController.java first 160 lines:\n'
sed -n '1,160p' apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java | cat -n
printf '\nAssistantService.java size and timeout/retrieval search:\n'
wc -l core/src/main/java/com/orgmemory/core/assistant/AssistantService.java
rg -n -C 10 'Future|Executor|submit|CompletableFuture|turnTimeout|timeout|queryPublishedSpaces|search\(|RetrievalAdmissionControl|future\.cancel|cancel\(|interrupt|completion|taken|take\(' core/src/main/java/com/orgmemory/core/assistant/AssistantService.java || trueRepository: kl3inIT/OrgMemory
Length of output: 9761
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'AssistantService.java full implementation:\n'
cat -n core/src/main/java/com/orgmemory/core/assistant/AssistantService.java
printf '\nUiMessageStream.java timeout implementation:\n'
wc -l apps/api/src/main/java/com/orgmemory/api/assistant/UiMessageStream.java
cat -n apps/api/src/main/java/com/orgmemory/api/assistant/UiMessageStream.java
printf '\nGraphRagKnowledgeRetrievalService relevant search/admission:\n'
wc -l core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalService.java
rg -n -C 12 'search\(|RetrievalAdmissionControl|completionQueue|published\(|queryPublishedSpaces|submit|Future|cancel' core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalService.java || trueRepository: kl3inIT/OrgMemory
Length of output: 20779
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'GraphRagKnowledgeRetrievalService.java outline:\n'
ast-grep outline core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalService.java --view digest || true
printf '\nGraphRagKnowledgeRetrievalService.java excerpts:\n'
wc -l core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalService.java
sed -n '1,320p' core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagKnowledgeRetrievalService.java | cat -n
printf '\nGraphRagRetrievalPolicy relevant defaults:\n'
cat -n core/src/main/java/com/orgmemory/core/knowledge/retrieval/GraphRagRetrievalPolicy.java
printf '\nRead-only behavioral probe for blocking queues/semaphores:\n'
python3 - <<'PY'
import threading, time
class Probe:
pass
def blocking_take():
p = Probe()
p.q = []
p.done = threading.Event()
def producer():
time.sleep(1)
p.q.append(1)
p.done.set()
threading.Thread(target=producer, daemon=True).start()
# Java BlockingQueue.take blocks until an element becomes available.
# This loop would also block in a real production use if the producer never comes.
for _ in range(5):
if p.done.is_set:
return p.q
time.sleep(0.1)
return "blocked"
try:
# Semaphore.acquire(0) also blocks until a permit is released by another thread.
from threading import BoundedSemaphore
s = BoundedSemaphore(0)
print("semaphore initial:", s.acquire(blocking=False))
print("take blocked:", threading.Thread(target=blocking_take).daemon == True)
except Exception as e:
print("probes:", repr(e))
PY
printf '\nSearch for any interrupt/cancel handling in assistant stream or timeout tests:\n'
rg -n -C 6 'interrupt|cancel\(|shutdownNow|turnTimeout|turn timed out|AssistantStreamAborted|blocks|acquir|take\(' apps/core apps/api core/src/test -g '*.java' || trueRepository: kl3inIT/OrgMemory
Length of output: 33633
Use bounded blocking waits for turn cancellation.
AssistantService.search() runs synchronously, then UiMessageStream.encode() applies the turn timeout only to the returned stream. Timeout cancellation does not interrupt the blocking RetrievalAdmissionControl.execute(Callable) queue or Future.get() wait because AssistantTurn does not hold the retrieval Future; it only holds the generated Flux. Run the search work through a bounded Future.get(timeout, TimeUnit), cancel the future on turn cancellation, and handle InterruptedException/CancellationException so admission permits are released or not consumed by abandoned turns.
🤖 Prompt for 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.
In
`@core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java`
around lines 23 - 30, Update RetrievalAdmissionControl.execute(Callable) and the
AssistantService.search() flow to run retrieval through a Future and await it
with the turn timeout via bounded Future.get(timeout, TimeUnit). On timeout or
turn cancellation, cancel the Future, handle InterruptedException and
CancellationException, and ensure abandoned work releases any acquired admission
permit instead of blocking indefinitely.
There was a problem hiding this comment.
Partially pre-existing, partially real — recording as a tracked follow-up rather than fixing here. Pre-existing: the turn timeout has never interrupted the synchronous AssistantService.search() — it applies to the returned stream only, before this PR too. New in this PR: an abandoned turn can additionally wait in the fair admission queue and will still consume a permit + one storage query after its timeout fired (bounded zombie work: at most one executePrepared per queued space, permits are always released in finally). At current production concurrency (45 turns/week, 4 permits) the waste window is small, and plumbing the turn deadline into the retrieval path crosses the Phase 1 scope boundary (ADR 0020 keeps timeout semantics unchanged). Added to the increment plan as a deferred finding so the Phase 2 compound-query port — which replaces this call path entirely — must address bounded, cancellable admission (deadline-aware acquire + cooperative cancellation between UiMessageStream and the retrieval future).
There was a problem hiding this comment.
@kl3inIT, acknowledged. The finding remains valid, but the Phase 1 scope decision is reasonable.
The current behavior bounds abandoned retrieval work, and RetrievalAdmissionControl releases permits reliably. Phase 2 must make admission deadline-aware and connect UiMessageStream cancellation to the retrieval Future.
🐇💭
✏️ Learnings added
Learnt from: kl3inIT
Repo: kl3inIT/OrgMemory PR: 292
File: core/src/main/java/com/orgmemory/core/knowledge/retrieval/RetrievalAdmissionControl.java:23-30
Timestamp: 2026-08-04T17:14:17.867Z
Learning: In the Java assistant retrieval flow, the turn timeout applies to the stream returned by synchronous `AssistantService.search()` and does not interrupt the retrieval call. `RetrievalAdmissionControl.execute(Callable)` uses a fair semaphore and releases permits in `finally`. The Phase 2 compound-query port must add deadline-aware admission and cooperative cancellation between `UiMessageStream` and the retrieval future.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
Phase 1 of ADR 0020 (retrieval admission control and pool right-sizing), per the active increment design and plan:
RetrievalAdmissionControl, envORGMEMORY_GRAPH_QUERY_ADMISSION_PERMITS): every snapshot storage query acquires a JVM-wide fair permit before enteringLightRagQueryEngine.executePrepared. Caps retrieval's total DB draw at 4 of 8 connections, removing the pool-exhaustion mode behind the bimodal 120 s turn timeouts (18% of production turns).future.get()barriers replaced with submit-all +ExecutorCompletionService(completion-order failure detection, request-order consolidation, unchanged fail-closed cancellation).No authorization path, cache identity, schema, or worker change. The compound multi-snapshot query is Phase 2, gated by ADR 0020 conditions 2–3.
Decision provenance: two-model architecture debate (Claude Fable 5 vs GPT-5.6-sol ultra, no-tools judge) consolidated in ADR 0020. Implementation by Codex
gpt-5.6-sol(high) under handoff; coordinator-reviewed.Verification
./gradlew --no-daemon clean test— PASS, 108 tasks, 8m17s (repeat on the final commit: PASS).topK=40/permit defaults, closed OTel attribute set, bounded Micrometer tags.Note for reviewers:
retrieve_snapshotdurations now include admission wait (timer starts before permit acquisition), which makes queue time observable but changes that metric's meaning across the deploy boundary.🤖 Generated with Claude Code
Summary by CodeRabbit