Skip to content

docs: add progressive disclosure documentation#2

Open
BenWeekes wants to merge 5 commits into
mainfrom
docs/progressive-disclosure
Open

docs: add progressive disclosure documentation#2
BenWeekes wants to merge 5 commits into
mainfrom
docs/progressive-disclosure

Conversation

@BenWeekes

Copy link
Copy Markdown
Collaborator

Summary

  • Add docs/ai/ progressive disclosure documentation following the ai-devkit standard
  • L0 repo card with identity and L1 index
  • 8 L1 structured summaries (setup, architecture, code map, conventions, workflows, interfaces, gotchas, security)
  • 2 L2 deep dives (profile configuration, agent lifecycle)
  • AGENTS.md at repo root as portable AI entry point
  • Updated CLAUDE.md to reference @AGENTS.md

Test plan

  • Verify all relative links between L0/L1/L2 resolve correctly
  • Verify L0 is under 50 lines, each L1 is 80-200 lines, total L1 under 1600 lines
  • Verify L2 files are self-contained with "When to Read This" callouts
  • Spot-check: can an agent answer "How do I add a new profile?" from L1 alone?

add L0 repo card, 8 L1 structured summaries, 2 L2 deep dives
(profile configuration, agent lifecycle), and AGENTS.md entry point
following the progressive disclosure standard
@digitallysavvy
digitallysavvy self-requested a review April 17, 2026 08:07

@digitallysavvy digitallysavvy left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared origin/docs/progressive-disclosure against main.

Scope Reviewed

  • New docs/ai/ progressive disclosure docs
  • New root AGENTS.md
  • CLAUDE.md update

Summary

The PR is structurally strong and mostly aligns with the stated ai-devkit format:

  • L0_repo_card.md is under 50 lines
  • All 8 L1 files exist and are within the 80-200 line target
  • Total L1 size is 802 lines, well under the 1,600 line budget
  • Both L2 deep dives are present and start with When to Read This
  • Relative links inside docs/ai/ resolve correctly
  • AGENTS.md provides a clear loading order and portable entry point

The main problems are content accuracy issues. Several docs describe configuration keys and runtime behavior that do not match the current codebase. Because this PR is documentation-only, those mismatches are the highest-risk part of the change.

Findings

1. TTS configuration key is documented incorrectly

Severity: High

The docs repeatedly tell readers to use TTS_API_KEY, but the backend reads TTS_KEY.

PR references:

  • docs/ai/L1/05_workflows.md:18
  • docs/ai/L1/deep_dives/profile_configuration.md:38
  • docs/ai/L1/deep_dives/agent_lifecycle.md:90-93

Code references:

  • simple-backend/core/config.py:76-88
  • simple-backend/core/agent.py:52-88
  • Existing repo docs already use TTS_KEY: README.md:97-102, simple-backend/README.md:103-106

Impact:

  • A reader following the new docs will set the wrong env var.
  • Agent creation will then fail or produce incomplete vendor config.

Suggested fix:

  • Replace all TTS_API_KEY mentions with TTS_KEY.
  • Keep naming consistent with the existing READMEs and actual config loader.

2. OpenAI MLLM vendor name is documented incorrectly

Severity: High

The deep dives describe the OpenAI realtime MLLM vendor as openai_realtime, but the implementation branches on openai.

PR references:

  • docs/ai/L1/deep_dives/profile_configuration.md:53
  • docs/ai/L1/deep_dives/agent_lifecycle.md:108

Code references:

  • simple-backend/core/agent.py:161-201
  • Existing backend docs use VOICE_MLLM_VENDOR=openai: simple-backend/README.md:62-68
  • Existing repo guide also uses VOICE_MLLM_VENDOR=openai: AGENT.md

Impact:

  • Readers may set VOICE_MLLM_VENDOR=openai_realtime.
  • That value will miss the vendor == "openai" branch and fall into the non-OpenAI path, producing the wrong payload shape.

Suggested fix:

  • Change openai_realtime to openai everywhere in the new docs.
  • If openai_realtime is meant to be supported, add code support first, then document it.

3. MCP server example uses the wrong field name

Severity: High

The docs show MCP entries with a url field, but build_mcp_servers() reads and mutates endpoint.

PR references:

  • docs/ai/L1/06_interfaces.md:118-127
  • docs/ai/L1/deep_dives/profile_configuration.md:75-77

Code references:

  • simple-backend/core/agent.py:217-243
  • Existing backend docs already use endpoint: simple-backend/README.md:398-402

Impact:

  • A user copying the PR example will produce invalid MCP config.
  • The backend appends the user/channel suffix to endpoint, so a url-only object will effectively become "/<user_id>".
  • This is likely to fail silently and is hard to debug.

Suggested fix:

  • Replace url with endpoint in all MCP examples.
  • Consider explicitly documenting that the backend appends /<user_id> to each configured endpoint.

4. CORS behavior is documented inaccurately

Severity: Medium

The new docs say CORS switches behavior based on whether the request includes an Authorization header, and one workflow tells readers to add route-level CORS headers manually. The actual server applies CORS globally in after_request() based on whether an Origin header is present.

PR references:

  • docs/ai/L1/04_conventions.md:71-75
  • docs/ai/L1/08_security.md:65-69
  • docs/ai/L1/05_workflows.md:23-26

Code references:

  • simple-backend/local_server.py:56-70

Impact:

  • Readers debugging cross-origin behavior will reason from the wrong trigger.
  • Contributors may add redundant route-level CORS code even though the app already centralizes it.

Suggested fix:

  • Reword the CORS sections to match the implementation:
    • if Origin is present, echo it and set Access-Control-Allow-Credentials: true
    • otherwise return *
  • Update the backend-route workflow to point contributors at the shared after_request() policy instead of telling them to add per-route headers.

5. The stated spot-check does not map to this repo

Severity: Low

The PR test plan says to spot-check whether an agent can answer “How do I add a new event type?” from L1 alone, but this repo does not appear to have an “event type” concept in the backend, frontend, or new docs.

Evidence:

  • rg -n "event type|event_type|eventType|new event" returned no hits outside vendored assets

Impact:

  • The test plan looks copied from a different repo template rather than tuned to this codebase.
  • That weakens confidence in the reported validation.

Suggested fix:

  • Replace that spot-check with a repo-specific question such as:
    • “How do I add a new TTS vendor?”
    • “How do I add a new profile?”
    • “How do I wire an MCP server?”

Validation Notes

I verified the following against the PR content:

  • docs/ai/L0_repo_card.md is 27 lines
  • L1 files are 94, 98, 91, 85, 88, 132, 126, and 88 lines
  • Total L1 lines: 802
  • Both L2 docs are present and include When to Read This
  • Relative links within docs/ai/ resolve

I also cross-checked the docs against the current code in:

  • simple-backend/core/config.py
  • simple-backend/core/agent.py
  • simple-backend/local_server.py
  • existing repo docs in README.md, simple-backend/README.md, and AGENT.md

Recommendation

Do not merge as-is.

The structure is good and worth keeping, but the doc inaccuracies around TTS keys, MLLM vendor names, MCP schema, and CORS behavior should be fixed before merge. Those are copy-pasteable setup instructions, so small wording errors here translate directly into broken user setups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants