docs: align omitted tools section with trading fixtures

[unseriousAI]

Closes #984

Changes

• Updated the Omitted Tools heading to remove the incorrect "no eval" claim.
• Replaced the trading mutations omission note to reflect that tests/evals/5_ord_schwab_* fixtures now exist.
• Clarified that meaningful trading-mutation eval runs still require live credentials/sandbox safety controls.

Test plan

• Verified the updated section text in tests/evals/ADK-testing-evals.md.
• Reviewed the file diff to confirm only the targeted documentation lines changed.

[claude]

Claude finished @unseriousAI's task —— View job

---

PR Review: Documentation Update for Omitted Tools Section

Todo List:

• Read the modified file to understand current documentation state
• Review the specific changes made in this PR
• Assess documentation quality, accuracy, and clarity
• Check consistency with project conventions
• Provide comprehensive review feedback

---

📋 Review Summary

This PR makes a precise documentation update that aligns the "Omitted Tools" section with the current state of the Schwab trading fixture evaluations. The changes are accurate and well-justified.

✅ Code Quality & Best Practices

Excellent - The changes demonstrate attention to detail:

• Precise scope: Only two lines changed, exactly what's needed
• Accurate content: Removes outdated "no eval" claim and correctly reflects that 5_ord_schwab_* fixtures now exist
• Clear communication: Explains the distinction between fixture existence and meaningful execution requirements

✅ Documentation Accuracy

Perfect alignment with codebase reality:

• ✅ Verified: 21 tests/evals/5_ord_schwab_* fixtures exist (confirmed via file glob)
• ✅ Accurate claim: Trading mutation tools DO have eval fixtures
• ✅ Important caveat preserved: Meaningful execution still requires live credentials/sandbox controls
• ✅ Reference maintained: Points to coverage gate documentation for justification

✅ Consistency & Style

Follows project conventions:

• Maintains existing markdown formatting and structure
• Uses consistent terminology ("fixture evals exist")
• Preserves safety warnings about live trading requirements
• Aligns with the detailed coverage matrix shown later in the document (lines 432-502)

🔍 Technical Verification

The changes accurately reflect the current state:

Before (line 380):

Omitted Tools (no eval — see coverage gate for justification)

After (line 380):

Omitted Tools (see coverage gate for justification)

Before (line 385):

require live credentials and execute real market orders or destructive cancellations; covered by integration journey tests.

After (line 385):

fixture evals exist in tests/evals/5_ord_schwab_*; meaningful execution still requires live credentials/sandbox safety controls because these calls place/modify/cancel orders.

🎯 Potential Improvements (Minor)

The changes are excellent as-is. For future consideration:

1. Cross-reference clarity: The updated text correctly points to tests/evals/5_ord_schwab_* — this aligns perfectly with the coverage matrix below (lines 432-502) which shows many Schwab tools marked as having evals.

2. Safety messaging: The revised text maintains the important distinction between "having fixtures" and "safe to run meaningfully" — this is crucial for user safety.

📊 No Issues Found

• ✅ No bugs or security concerns
• ✅ No performance impacts (documentation only)
• ✅ Test coverage: Documentation change only, no code logic affected
• ✅ No breaking changes

🎯 Recommendation

APPROVE - This is a high-quality documentation update that:

• Fixes outdated information
• Maintains important safety warnings
• Accurately reflects the current test fixture landscape
• Demonstrates careful attention to detail

The PR successfully closes issue #984 by aligning documentation with the reality that Schwab trading evaluation fixtures do exist, while preserving the critical safety context.

---

[jwesleye]

AGENTSHORE_CODE_REVIEW
head_sha: 684387e
status: PASS
spec_compliance: PASS
blocking_findings: 0
non_blocking_findings: 1

Summary

Accurate, well-scoped documentation fix that correctly reflects the existence of 5_ord_schwab_* eval fixtures.

Blocking

None

Suggestions

1. tests/evals/ADK-testing-evals.md lines 382–385 — The section intro still reads "intentionally excluded from ADK eval coverage" while the trading mutations bullet now acknowledges that fixture evals exist. The statement is technically accurate (the tools remain in EVAL_OMISSION_ALLOWLIST) but a reader encountering both lines in sequence may find it contradictory. A small follow-up clarification — e.g., "excluded from the automated coverage gate" — would remove the ambiguity. Non-blocking; the current text is better than before.