Make DigitalBrain observable and product-aware - #54
Conversation
Adds a self-cleaning Aspire proof for resource health, the exact MCP catalog, a real Gemma4 turn, durable transcript correlation, and telemetry hygiene. Updates the repository status to the measured observable product state.\n\nNo unconsumed product surface was added; the verifier is linked from the contributor workflow. All capability claims were checked by the script's live successful run. The three pre-existing Flutter generated-file changes remain unstaged and untouched.
Adds responsive Chat, Activity, and Brain surfaces over one shared durable chat projection. Activity exposes sequence and command facts without message content, while Brain describes only the product's measured proposal-gated capabilities.\n\nEvery added surface has a current UI consumer. Claims were checked by Flutter/Dart analysis and tests, the Windows Release build, and the 253-test root MTP gate. The three pre-existing Flutter generated-file changes remain unstaged and untouched.
Consumers: topology polling, capability rendering, inspector state, activity facts, and pulse visualization all consume this slice today. Verified: wire/client/shell analyses and tests, UI edge tests, Release root build and 255-test MTP gate, and Windows Release build. Preserved: three pre-existing generated Windows Flutter files remain unstaged.
Consumers: the UI topology resolves an Aspire-projected client manifest and Flutter renders only explicit product capabilities. Verified: 258-test root MTP gate, all client analyses/tests, Windows Release build, and live Gemma4 verification with six modules and two configured capabilities. Preserved: three pre-existing generated Windows Flutter files remain unstaged.
Project model-specific client-safe LLM features so the Gemma4-backed assistant is advertised only when Gemma4 is actually configured. Cover non-Gemma rejection and Gemma composition projection.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
LeftTwixWand
left a comment
LeftTwixWand
left a comment
There was a problem hiding this comment.
Extreme multi-agent PR review — synthesis
Verdict: COMMENT (no P0/P1 after adversarial calibration)
Agents: 100/100 child agents completed (Phase1:10 + Phase2:60 + Phase3:24 + Phase4:6). Coordinator not counted.
Complete PR diff reviewed: master...product/observable-digitalbrain (83 files, +4182/−369, 10 commits, HEAD 89e42a6).
Findings posted after adversarial verification (≥2 validators + Phase-4 audits):
| Pri | ID | Title |
|---|---|---|
| P2 | F-005 | Assistant always registers enrichment tool; catalog gates separately |
| P2 | F-007 | AddStandardResilienceHandler applies to MCP HttpClient factory clients |
| P2 | F-008 | Ollama HttpClient default ~100s under 5m ResponseTimeout |
| P2 | F-011 | send_chat_message mints CommandId server-side; not retry-safe after timeout |
| P3 | F-001 | UseOpenTelemetry does not pin EnableSensitiveData=false |
| P3 | F-009 | MCP max wait 180s vs neuron/agent LongRunning 5m |
| P3 | F-010 | verify-product.ps1 final aspire stop ignores exit code |
Discarded candidates (representative): ~40+ raw Phase-2 candidates collapsed; 12 clustered IDs reduced further. Discarded includes: desktop SSE no-reconnect (pre-existing, not PR-introduced), list_active_neurons silo dump (pre-existing), Guid "N"/"n" casing (not a real bug), empty AssistantAnswered hang (ChatNeuron line not PR-changed; exposed by MCP wait but no tight PR anchor), enrichment “approval” overclaim (copy already says proposal-only).
Validation commands actually run:
gh auth status— OK (LeftTwixWand)gh pr view 54 --json ...— OKgit rev-parse HEAD/git merge-base master HEAD/git diff --stat master...HEAD— OKdotnet --version— 11.0.100-preview.6.26359.118- Static re-reads of AIClients, ServiceDefaults, DigitalBrainMcpTools, Assistant, ChatNeuron, verify-product.ps1
- 100 agent tool-using reviews (read/grep/gh/git); no claim of green root
dotnet testor liveverify-product.ps1in this review run
Checks not performed: full Release root gate; live Gemma4 turn; aspire resource health in this session.
Confirmation: Every posted finding survived multi-agent adversarial challenge and Phase-4 intersection (anchors + evidence + severity calibration). F-006 was rejected at Phase-4 line-anchor audit.
| completed update. If authentication or a provider is unavailable, report that limitation. | ||
| """; | ||
|
|
||
| protected override IReadOnlyList<CapabilityTool> Tools => |
There was a problem hiding this comment.
[P2] Assistant always registers enrichment tool; catalog gates separately
Trigger: Any composition with OSBehaviorsModule but without Google/Salesforce modules or ConfiguredFeatures (or future optional provider install).
Impact: Model always receives �nrich_account_from_email and system text claiming that external action. BrainCapabilityCatalog only advertises �ccount-enrichment.gmail-salesforce-description when modules + features match — dual capability truth.
Why permitted: PR-added Instructions hardcode the tool as always available; Tools is an unconditional list. Catalog is UI-only and never consulted by Agent.Respond.
Smallest fix: Gate Tools/Instructions with the same module/feature predicates used by BrainCapabilityCatalog (without importing Flutter.Http), or compose separate assistants for product vs thin graphs.
| builder.Services.AddServiceDiscovery(); | ||
| builder.Services.ConfigureHttpClientDefaults(http => | ||
| { | ||
| http.AddStandardResilienceHandler(); |
There was a problem hiding this comment.
[P2] AddStandardResilienceHandler applies to MCP factory HttpClients
Trigger: Product silo calls AddServiceDefaults(); Google/Salesforce open MCP sessions via IHttpClientFactory.CreateClient(""DigitalBrain.Mcp"") after services.AddHttpClient().
Impact: Standard resilience defaults (including retries on all HTTP methods and short total/attempt timeouts) wrap outbound MCP POSTs. Non-idempotent tool calls can be double-submitted on transient 5xx/timeouts.
Why permitted: New ServiceDefaults uses ConfigureHttpClientDefaults without a named MCP client that removes or retunes resilience.
Smallest fix: Register AddHttpClient(McpRuntime.HttpClientName) with RemoveAllResilienceHandlers() (or disable unsafe-method retries and set MCP-appropriate timeouts). Keep generic defaults for other clients.
| var tag = configuration[$"{ConfigurationRoot}:Ollama:{modelName}:Model"] ?? defaultTag; | ||
|
|
||
| return new OllamaApiClient(endpointUri, tag); | ||
| return new ChatClientBuilder(new OllamaApiClient(endpointUri, tag)) |
There was a problem hiding this comment.
[P2] Ollama client keeps default ~100s HTTP timeout under 5m grain budget
Trigger: Product chat uses Gemma4 via
ew OllamaApiClient(endpointUri, tag) while IAgent.Respond declares [ResponseTimeout(NeuronCallTimeouts.LongRunning)] (5 minutes). Non-streaming GetResponseAsync is bounded by HttpClient.Timeout default (100s).
Impact: Slow model generations fail at the HTTP layer before the declared Orleans response budget applies; core chat path can fail on long but valid turns.
Why permitted: PR wraps Ollama in ChatClientBuilder/UseOpenTelemetry but still constructs OllamaApiClient without an HttpClient whose Timeout matches LongRunning.
Smallest fix: Pass
ew HttpClient { BaseAddress = endpointUri, Timeout = TimeSpan.FromMinutes(5) } (or slightly higher) into OllamaApiClient.
| JournalKind.Outgoing, | ||
| afterSequence: 0); | ||
| var commandId = CommandId.New(); | ||
|
|
There was a problem hiding this comment.
[P2] send_chat_message mints CommandId server-side; not retry-safe after timeout
Trigger: Client/operator retries the tool after a wait timeout (or transport drop) once IChat.Send has already committed UserMessaged.
Impact: Each call does CommandId.New() then Send. A retry creates a second durable user turn and can start a second model chain, while the first turn may still complete.
Why permitted: Tool API has no client-supplied command/idempotency key; timeout only aborts the journal poll after Send.
Smallest fix: Accept optional commandId; if that command’s UserMessaged already exists, skip Send and only wait. Document that retries without a command id are not idempotent.
|
|
||
| return new OllamaApiClient(endpointUri, tag); | ||
| return new ChatClientBuilder(new OllamaApiClient(endpointUri, tag)) | ||
| .UseOpenTelemetry(sourceName: $"{TelemetrySource}.{modelName}") |
There was a problem hiding this comment.
[P3] UseOpenTelemetry does not pin EnableSensitiveData = false
Trigger: Process env OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=true (or equivalent) with OTLP export enabled.
Impact: MEAI can export gen_ai.input.messages / gen_ai.output.messages (and related sensitive tags), violating the product content-hygiene rule that GenAI telemetry excludes prompt/response bodies. Defaults are safe; the pin is missing defense-in-depth.
Why permitted: Both Ollama and OpenAI pipelines call .UseOpenTelemetry(sourceName: …) with no configure callback.
Smallest fix: .UseOpenTelemetry(sourceName: …, configure: static c => c.EnableSensitiveData = false) on both builders. Keep verify-product attribute absence check.
| [Description("User prompt for Llama 3.2")] string prompt) | ||
| private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(50); | ||
| private const int DefaultTimeoutSeconds = 120; | ||
| private const int MaximumTimeoutSeconds = 180; |
There was a problem hiding this comment.
[P3] MCP max wait (180s) is shorter than neuron/agent LongRunning (5m)
Trigger: Model+tool turn legally runs between 181s and 300s under NeuronCallTimeouts.LongRunning.
Impact: send_chat_message throws TimeoutException while the grain turn may still complete and journal AssistantResponded later — false timeout relative to the product long-running budget.
Why permitted: MaximumTimeoutSeconds = 180 is independent of NeuronCallTimeouts.LongRunning = ""00:05:00"".
Smallest fix: Align maximum (and default, if desired) with LongRunning, or document explicitly that MCP wait is a stricter operator cap than the grain budget.
| Pop-Location | ||
|
|
||
| if ($started -and -not $KeepRunning) { | ||
| & aspire stop --apphost $appHost --non-interactive |
There was a problem hiding this comment.
[P3] Final �spire stop ignores exit code
Trigger: �spire stop fails after a green proof ($started true, not -KeepRunning).
Impact: Script can exit success while AppHost (and fixed MCP port 5000 / Flutter process) remains running, breaking the “self-cleaning oracle” contract and the next local run.
Why permitted: Proof steps use Invoke-Checked; the inally stop is bare & aspire stop without $LASTEXITCODE handling.
Smallest fix: On the success path, check stop’s exit code (without masking an earlier try failure). Fail the script if stop fails after a successful proof.
Agreed repair planAfter reviewing the diff, current checks, and the updated requirements, this is the implementation plan:
Existing thread disposition
|
Collapse needlessly chopped method/ctor/new argument lists across product, OS, and fixture C#. Long or intentional multi-line forms left alone. Formatting only; no behavior changes.
| public static string FlutterShellDirectory => Path.Combine( | ||
| FlutterClientDirectory, | ||
| "shell"); | ||
| public static string FlutterShellDirectory => Path.Combine(FlutterClientDirectory, "shell"); |
Stream model turns with GenAI usage telemetry and AppHost-controlled sensitive capture. Make MCP chat retries idempotent and introspection owner-scoped. Remove inferred capabilities, mirror tests, and the forbidden PowerShell verifier; replace the latter with an explicit live product proof. No-consumer check: none; every addition is consumed by the AppHost, MCP flow, or live product path. Unverified-claim check: none; Release build, the 252-test root gate, the explicit Gemma product proof, and all Dart/Flutter gates were run. Unowned-change check: preserved the user's formatting pass; unchanged generated Flutter files were not staged.
|
Applied the repair in What changed:
Fresh verification on the committed tree:
|
| var http = new HttpClient | ||
| { | ||
| BaseAddress = endpointUri, | ||
| Timeout = RequestTimeout, | ||
| }; |
| Timeout = RequestTimeout, | ||
| }; | ||
|
|
||
| return new ChatClientBuilder(new OllamaApiClient(http, tag)) |
| catch | ||
| { | ||
| } |
| catch | ||
| { | ||
| } |
| catch | ||
| { | ||
| } |
| catch | ||
| { | ||
| } |
| { | ||
| var directory = new DirectoryInfo(AppContext.BaseDirectory); | ||
| while (directory is not null | ||
| && !File.Exists(Path.Combine(directory.FullName, "DigitalBrain.slnx"))) |
Give journal watch calls the same long-running Orleans response contract as journal reads, and allow AppHost graph cleanup the five-minute resource budget used by startup and operations. No-consumer check: none; the existing SSE journal path and AppHost fixture cleanup consume these boundaries. Unverified-claim check: none; both owning suites and the exact solution-wide CI command were run. Unowned-change check: none.
Delete the redundant per-resource stop-command pass. DistributedApplication.StopAsync and DisposeAsync own graph teardown; individual stop commands are test actions and returned false failures on Linux CI even when the graph and exclusive lease were released. No-consumer check: none; this removes an unneeded layer rather than adding surface. Unverified-claim check: none; the host suite passed twice and the exact 252-test CI command passed. Unowned-change check: none.
|
CI follow-up after the review fixes:
Verification on final SHA
The branch is clean and the remote head matches this SHA. |
1 participant
Summary
ai.EnableSensitiveDataWhy
The product needed an observable end-to-end path whose claims come from live behavior rather than source configuration. A configured module or model does not prove provider credentials, authorization, or readiness, and source/reflection tests do not prove runtime behavior.
This change keeps journals as the causal source of truth, makes interrupted MCP chat calls safe to resume, and emits useful GenAI telemetry. Sensitive prompt/response capture remains off by default and is enabled explicitly by the product AppHost only when its environment is Development.
Impact
Users get an honest DigitalBrain identity, explicit external-action boundaries, durable retry behavior, and a Flutter workspace for Chat, Activity, and live Brain topology. Operators get correlated journal and OpenTelemetry evidence without exposing raw silo placement. Production telemetry remains content-safe by default; local Development can opt into prompt/response capture for diagnosis.
Validation
dotnet build DigitalBrain.slnx -c Release --verbosity minimal— exit 0dotnet test DigitalBrain.slnx -c Release --no-build --verbosity minimal— 252 total, 250 passed, 2 explicit skips, 0 faileddotnet test os/tests/DigitalBrain.OS.Product.Tests/DigitalBrain.OS.Product.Tests.csproj -c Release -- -explicit only— 1/1 passed against real Gemma/Aspire; verified streaming response, retry deduplication, transcript/journal correlation, owner-scoped introspection, token usage, finish reason, and Development prompt/response telemetrygit diff --checkclean; no*.ps1files remain