docs: narrow the getAgent() result in the agents guide samples - #3571
Conversation
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe agent guide now checks ChangesAgent example safety
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
a097c2a to
af6465a
Compare
|
Rebased onto current
Nothing caught that: Re-verified red-before-green after the edits: stripping both guards out of the guide still fails the test with Scope is unchanged — still the agents guide plus its regression test. Still deliberately out of scope, as flagged in the description, all verified against this tree:
Those want their own change with the test widened to cover the guides it actually asserts on; the test here is scoped to the agents guide precisely so it does not claim coverage it does not have. A separate note for whoever picks that up: the |
|
@coderabbitai review |
|
|
CI is green on The first run after the rebase went red on three checks, all from one flake: Note for anyone matching this against the usual report: the flake landed in coverage shard 2/8 this time, not 8/8. Sharding is by file distribution, so the rebase onto current Re-ran the failed jobs on the identical commit and all three passed — shard 2/8 in 2m15s, then The local |
af6465a to
5b18611
Compare
getAgent() returns Agent | undefined, but both getAgent() samples in the agents guide called .generate() on the result directly. Pasted verbatim into a project scaffolded by veryfront init, they fail typecheck with TS18048 under the "strict": true tsconfig the scaffolder itself writes. Add the missing narrowing guard to both samples and state why it is there.
Follow-ups on the same two files: - The note landed after a sentence ending in a colon, so the colon dangled in front of a prose paragraph instead of the code fence it announced. Fold it into the lead-in sentence. Uses ASCII punctuation, which validate-public-docs.ts enforces. - The new assertion indexed regex capture groups without narrowing, so the file stopped type-checking under the repo's noUncheckedIndexedAccess. The docs lane runs these tests with --no-check and the test-typecheck ratchet only walks src/ and cli/, so nothing caught it. Guard both captures.
5b18611 to
c037656
Compare
Found during a DX dogfood walk of https://veryfront.com/docs/code/guides/agents, following the published docs literally as a new developer would.
Symptom
Both
getAgent()samples in the agents guide (the "Non-streaming response" one and the "Verify it worked" one) are given verbatim as:Paste either into a project created by
veryfront initand run the typechecker:The samples work at runtime, so the failure only shows up once the developer runs the typechecker the scaffolder configured for them. The guide never mentions the narrowing and offers no guard.
Root cause
getAgent()returnsAgent | undefined(src/agent/composition/composition.ts:216) because the id may not be registered. The scaffolder writes"strict": trueinto the project tsconfig (cli/commands/init/config-generator.ts:106, and everycli/templates/files/*/tsconfig.json). UnderstrictNullChecks, calling a method on the result without narrowing is an error. The samples predate that and were never updated.Reproduced against this tree, not just the published build:
Fix
Add
if (!agent) throw new Error("Agent not found: assistant");to both samples, plus one sentence sayinggetAgent()returnsAgent | undefinedand why the guard is there. No API change.Regression test
tests/docs/guide-content.test.ts— "narrows the possibly-undefined getAgent() result in agents guide samples".It lives there because
docs/guides/agents.mdin this repo is the source of truth for the published page (veryfront-docsdocs/code/guides/agents.mdis synced from it and hand edits there are overwritten), andtests/docs/guide-content.test.tsis the existing home for guide-content contracts. It is also already wired intodeno task docs:validate, so it runs in theci (lint)lane.The test parses the
ts/tsxfences in the guide, finds everyconst X = getAgent(...)binding that is later dereferenced, and fails unless anif (!X)guard appears before the first use. That is a structural check on the actual defect, not a magic-string assertion, so a future sample that drops the guard fails too.Confirmed it fails before the fix for the right reason:
and passes after.
deno task docs:validate's guide validators,check-doc-links.ts,deno fmt --checkanddeno lintare clean on the touched files.Noted, not fixed here
The same unguarded pattern exists in
docs/guides/memory-and-streaming.md(3 samples) anddocs/guides/multi-agent.md(agentAsTool(researcher, ...)passes a possibly-undefined value into anAgentparameter). Those are outside this finding's scope and are left for a separate change; the test added here is deliberately scoped to the agents guide so it does not silently claim coverage it does not have.Summary by CodeRabbit
Documentation
Tests