fix(tinyagents-audit): execute §5 fix-in-place items — provider diagnostic, anchor coupling, dead-read clarifications

[senamakel]

Summary

Execute the safe, behavior-preserving §5 "bugs, gaps & improvements found during the audit" items from docs/tinyagents-port-plan.md — the fix-in-place candidates independent of the port itself. Six focused commits:

• fix(inference): the default Provider::stream_chat_with_history named its unsupported-streaming error "unknown" — now uses the trait's telemetry_provider_id() so the diagnostic names the real provider (+ test).
• refactor(inference): the Sentry before_send rejection filter hardcoded a literal that duplicates factory::NO_MODEL_CONFIGURED_ANCHOR — reference the const directly so they compile-couple (was test-guarded only).
• docs(inference): recommend_tier reads RAM but always returns MVP_MAX_TIER — documented as the intentional MVP cap (test already pins the non-scaling contract) so it stops reading as dead logic.
• docs(agent_orchestration): clarified that SteerError::NotOwned is not dead — the agent-tool steer path enforces ownership and returns it; only the trusted RPC path skips the check.
• refactor(tools): dropped a stale #[allow(dead_code)] on ToolScope::AgentOnly (it has three real constructors) and documented that it currently behaves like All.
• docs(tools): re-attached a mis-placed test doc-comment in orchestrator_tools.rs to the slug-collision test it actually describes.

Problem

The port-plan audit catalogued a set of localized defects and misleading/dead-reading code in inference/, tools/, and agent_orchestration/ (plan §5). They are independent of the multi-phase crate consolidation and worth fixing in place — several would otherwise be deleted-in-place with their host anyway, but until then they mislead readers (e.g. a "unknown does not support streaming" diagnostic, a recommend_tier that looks broken, a NotOwned arm that looks dead).

Solution

Only the behavior-preserving subset is applied here; each genuine behavior/API decision the audit raised is deliberately deferred to its planned phase and left documented, not guessed:

• RAM→tier scaling in recommend_tier (would violate the MVP cap) — not implemented.
• RPC-path ownership enforcement for SteerError::NotOwned (would change the JSON-RPC payload/trust model, which the plan forbids during the port) — deferred to Phase 4's SteeringRegistry.
• ToolScope::AgentOnly vs All reconciliation and is_concurrency_safe adoption (needs upstream tinyagents parallel dispatch) — deferred to Phase 2 / upstream.

The one observable change (the streaming-unsupported diagnostic text) is covered by a new co-located unit test asserting it names the provider and no longer says "unknown".

Submission Checklist

• Tests added or updated — new default_stream_chat_with_history_names_provider_when_unsupported covers the only behavior change (diagnostic text). The remaining commits are doc/annotation/refactor with no behavior delta.
• Diff coverage ≥ 80% — the only changed logic line (provider_name = self.telemetry_provider_id()) is directly exercised by the new test; the rest are comments/annotations. Full validation deferred to CI.
• Coverage matrix updated — N/A: no user-facing feature rows (internal diagnostics/comments/refactors).
• All affected feature IDs listed under ## Related — N/A.
• No new external network dependencies introduced — confirmed, none.
• Manual smoke checklist updated — N/A: no release-cut surface change.
• Linked issue closed — N/A (audit fix-in-place batch; see ## Related).

Impact

• Runtime: desktop core (Rust). No JSON-RPC surface, tool catalog, provider grammar, or execution-model change.
• Observability: the unsupported-streaming error and the [local_ai] recommended model tier context are now more actionable; the config_rejection anchor can no longer silently drift from Sentry's filter.
• Security: none — ownership/gating semantics are unchanged (the NotOwned and AgentOnly decisions that would touch trust/gating are deferred).

Related

• Closes: N/A (executes §5 of the docs/tinyagents-port-plan.md audit; not a tracked issue)
• Follow-up PR(s)/TODOs: deferred design-calls above (Phase 2 tool-model reconciliation; Phase 4 SteeringRegistry ownership). The port itself lands separately (PR feat(tinyagents): Phase 0/1 port cutover — pin v1.7.1, crate-back SchemaCleanr / model-context / observed stream #4539, Phase 0/1).

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: fix/tinyagents-audit-fixes
• Commit SHA: 1dfef9d963889a27ee8730a07f04d5cd612d1f20

Validation Run

• pnpm --filter openhuman-app format:check — N/A (no app/src changes)
• pnpm typecheck — N/A (no TypeScript changes)
• Focused tests: authored a co-located unit test for the diagnostic change; cargo fmt applied.
• Rust fmt/check (if changed): cargo check --lib run locally on the branch (see notes); cargo fmt clean.
• Tauri fmt/check (if changed): N/A (core-only change); full validation on CI.

Validation Blocked

• command: full cargo test suite
• error: none — deferred to GitHub runners per the requester; local validation bounded to cargo check --lib + cargo fmt.
• impact: CI (CI Lite on this PR) is the authoritative gate.

Behavior Changes

• Intended behavior change: only the human-readable unsupported-streaming diagnostic (names the provider instead of "unknown").
• User-visible effect: none in normal operation (the diagnostic surfaces only when a provider without streaming support is asked to stream).

Parity Contract

• Legacy behavior preserved: tier selection, steering/ownership semantics, tool gating, JSON-RPC surface — all unchanged.
• Guard/fallback/dispatch parity checks: recommend_tier non-scaling contract still pinned by its test; config_rejection anchor still guarded by the factory_tests.rs round-trip.

Duplicate / Superseded PR Handling

• Duplicate PR(s): none.
• Canonical PR: this one.
• Resolution: N/A.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR consists of documentation and comment clarifications across several files, plus one behavioral fix: the default Provider::stream_chat_with_history diagnostic now uses telemetry_provider_id() instead of the hardcoded literal "unknown", with new tests validating this. A rejection-phrase matcher was also updated to reference a shared anchor constant instead of an inlined string.

Changes

Comments, Diagnostics, and Shared Constants

Layer / File(s)	Summary
Comment clarifications src/openhuman/agent_orchestration/subagent_control.rs, src/openhuman/inference/presets.rs	Expands inline comments explaining why the NotOwned arm is unreachable in handle_subagent_steer and documenting the MVP tier-capping behavior in recommend_tier; no logic changes.
Provider streaming diagnostic fix src/openhuman/inference/provider/traits.rs, src/openhuman/inference/provider/traits_tests.rs	Changes the default streaming-unsupported error message to use self.telemetry_provider_id() instead of "unknown"; adds a NamedNoStreamProvider test helper and async test asserting the diagnostic names the provider correctly.
Shared rejection anchor constant src/openhuman/inference/provider/config_rejection.rs	Replaces a hardcoded "empty model id" substring match with the shared NO_MODEL_CONFIGURED_ANCHOR constant and updates the related comment.
Doc reordering and scope clarification src/openhuman/tools/orchestrator_tools.rs, src/openhuman/tools/traits.rs	Moves a test documentation comment to precede its matching test, and expands ToolScope::AgentOnly documentation while removing an unneeded #[allow(dead_code)] attribute.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• TinyAgents migration (v0.58.7 → main) parity & bug-fix tracker — harness audit findings #4470: The handle_subagent_steer comment update about NotOwned/steering ownership consolidation aligns with this issue's steering-state/forwarding work.

Possibly related PRs

• tinyhumansai/openhuman#4506: Introduces/uses telemetry_provider_id across provider implementations, directly related to its use in the streaming diagnostic fix here.

Suggested labels: `bug`

Suggested reviewers: `sanil-23`, `M3gA-Mind`

Poem

A hop, a tweak, a comment refined,
"unknown" gives way to a name well-defined.
Anchors shared instead of strings alone,
Slugs shuffled gently, docs newly grown.
This bunny approves with a happy twitch of the nose 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the PR as a set of §5 fix-in-place updates covering provider diagnostics, anchor coupling, and documentation clarifications.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/inference/provider/traits.rs (1)

732-739: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Stale comment and minor format-string inconsistency.

Line 732's comment ("we need to convert to owned strings") was accurate when the code literally did "unknown".to_string(); now that provider_name comes from telemetry_provider_id() (already an owned String), the rationale is stale. Also, line 739's format! uses positional args while nearby code in this file (e.g. line 535) uses inlined args ({e} style).

♻️ Optional cleanup

-        // For default implementation, we need to convert to owned strings
         // This is a limitation of the default implementation. Name the real
         // provider via its telemetry slug so the diagnostic is actionable
         // instead of the useless literal "unknown".
         let provider_name = self.telemetry_provider_id();

         // Create a single empty chunk to indicate not supported
-        let chunk = StreamChunk::error(format!("{} does not support streaming", provider_name));
+        let chunk = StreamChunk::error(format!("{provider_name} does not support streaming"));

🤖 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 `@src/openhuman/inference/provider/traits.rs` around lines 732 - 739, Update
the default streaming-not-supported path in traits.rs by removing the stale
comment about converting to owned strings now that provider_name is sourced from
telemetry_provider_id(), and make the StreamChunk::error formatting consistent
with the rest of the file by using the inline variable style instead of a
positional format argument. Keep the fix localized to the default implementation
that builds provider_name and the error chunk.

🤖 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.

Nitpick comments:
In `@src/openhuman/inference/provider/traits.rs`:
- Around line 732-739: Update the default streaming-not-supported path in
traits.rs by removing the stale comment about converting to owned strings now
that provider_name is sourced from telemetry_provider_id(), and make the
StreamChunk::error formatting consistent with the rest of the file by using the
inline variable style instead of a positional format argument. Keep the fix
localized to the default implementation that builds provider_name and the error
chunk.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9b6bbb06-201b-4d2d-8dc1-5e7e389f85b5

📥 Commits

Reviewing files that changed from the base of the PR and between 9191cd4 and 1dfef9d.

📒 Files selected for processing (7)

• src/openhuman/agent_orchestration/subagent_control.rs
• src/openhuman/inference/presets.rs
• src/openhuman/inference/provider/config_rejection.rs
• src/openhuman/inference/provider/traits.rs
• src/openhuman/inference/provider/traits_tests.rs
• src/openhuman/tools/orchestrator_tools.rs
• src/openhuman/tools/traits.rs