Skip to content

Review and update callbacks documentation#697

Merged
Timonwa merged 3 commits into
mainfrom
695-review-and-test-the-adk-ts-callbacks-documentation
May 6, 2026
Merged

Review and update callbacks documentation#697
Timonwa merged 3 commits into
mainfrom
695-review-and-test-the-adk-ts-callbacks-documentation

Conversation

@Timonwa

@Timonwa Timonwa commented May 6, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

Rewrites the callbacks documentation section for clarity, accuracy, and structure. The changes cover the overview (index.mdx), callback types reference (types.mdx), and practical patterns guide (callback-patterns.mdx), as well as a minor navigation update (meta.json).

Key improvements:

  • Reorganized content around user goals rather than API surface
  • Replaced verbose explanations with minimal, focused code examples
  • Added guardrails/policy enforcement, caching, state management, logging, and conditional execution patterns
  • Aligned terminology and code samples with current ADK-TS API (AgentBuilder, CallbackContext, ToolContext)
  • Removed inaccurate or outdated descriptions

Related Issue

Closes #695

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)
  • Tests
  • Other (please describe):

How Has This Been Tested?

  • Reviewed all updated MDX pages for correctness against the source code
  • Verified code examples use the current ADK-TS API patterns
  • Checked navigation renders correctly via meta.json

Checklist

  • My code follows the code style of this project
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • My changes generate no new warnings
  • I have checked for potential breaking changes and addressed them

Screenshots (if applicable)

N/A — documentation-only changes

Additional Notes

This is part of the ongoing documentation review and improvement initiative for the ADK-TS framework.

- Updated the introduction to emphasize the lifecycle and functionality of callbacks in ADK-TS.
- Enhanced the descriptions of callback firing points and their purposes.
- Improved the callback registration section with clearer examples and return value explanations.
- Added a visual representation of the callback flow using Mermaid diagrams.
- Streamlined the examples for each callback type to demonstrate practical usage.
- Removed redundant content and ensured consistency in terminology across the documentation.
- Adjusted the meta.json to remove the index page from the callbacks section.
@changeset-bot

changeset-bot Bot commented May 6, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 47504b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
adk-typescript-docs Ready Ready Preview May 6, 2026 4:41pm
adk-web Ready Ready Preview May 6, 2026 4:41pm

Request Review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the ADK-TS callback documentation, streamlining the content and updating examples to use the AgentBuilder API. The changes include more concise descriptions of the six lifecycle hooks and the addition of practical design patterns for guardrails, caching, and state management. Feedback highlights a recurring typo in the model name (gemini-2.5-flash), potential runtime errors due to missing safety checks on llmRequest.contents, and incorrect string concatenation logic for systemInstruction which is typically an object. Additionally, the Mermaid lifecycle diagram in the index file needs adjustment to accurately reflect the iterative nature of model and tool calls.

Comment thread apps/docs/content/docs/framework/callbacks/index.mdx Outdated
Comment thread apps/docs/content/docs/framework/callbacks/callback-patterns.mdx
Comment thread apps/docs/content/docs/framework/callbacks/callback-patterns.mdx Outdated
Comment thread apps/docs/content/docs/framework/callbacks/callback-patterns.mdx Outdated
Comment thread apps/docs/content/docs/framework/callbacks/types.mdx Outdated
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Adebesin-Cell

Copy link
Copy Markdown
Member

@claude review

@claude

claude Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Adebesin-Cell's task —— View job


Callbacks Documentation Review

Todo List:

  • Read and analyze all 4 changed documentation files
  • Review content structure and organization
  • Check code examples for technical accuracy
  • Verify ADK-TS API usage consistency
  • Address automated review feedback points
  • Provide comprehensive review feedback

Overall Assessment

The PR significantly improves the callbacks documentation with clearer organization, concise examples, and practical patterns. However, there are several technical issues that need to be addressed:

Critical Issues

1. Invalid Model Name Throughout ⚠️

The model name gemini-2.5-flash appears 114 times across the codebase but this model does not exist. Based on the codebase, valid options are:

  • gemini-1.5-flash (most common in codebase)
  • gemini-2.0-flash (also used)

Files affected: index.mdx:65, types.mdx:37,112,144,305, callback-patterns.mdx:31,84,127,173,215,270,326

2. Unsafe Array Access ⚠️

In callback-patterns.mdx:40, the code accesses llmRequest.contents directly without null safety:

// Current (unsafe)
const text = llmRequest.contents
  .flatMap(c => c.parts ?? [])

// Should be (safe)  
const text = (llmRequest.contents ?? [])
  .flatMap(c => c.parts ?? [])

3. SystemInstruction Type Handling Issues ⚠️

In multiple places, systemInstruction is treated as a string when it can be a Content object:

In callback-patterns.mdx:227-228:

// Problematic - will fail if systemInstruction is an object
llmRequest.config.systemInstruction = 
  `${llmRequest.config.systemInstruction ?? ""}\nUser tier: ${userTier}`.trim();

In types.mdx:138-139:

// Same issue here
llmRequest.config.systemInstruction =
  `${llmRequest.config.systemInstruction ?? ""}\nLanguage: ${lang}`.trim();

Proper handling should check the type:

if (llmRequest.config) {
  const currentInstruction = llmRequest.getSystemInstructionText() ?? "";
  llmRequest.config.systemInstruction = `${currentInstruction}\nUser tier: ${userTier}`.trim();
}

4. Mermaid Diagram Logic Issue ⚠️

In index.mdx:16-24, the lifecycle diagram shows afterModelCallback and afterToolCallback leading directly to afterAgentCallback, but they should show loops back to the agent logic since multiple model/tool calls can occur in a single agent run.

Minor Issues

5. Inconsistent Import Patterns

Some examples import types that aren't used, while others are missing necessary imports. For example, in callback-patterns.mdx:288, LlmRequest is imported but not used in that function.

6. Navigation Update

The meta.json change removing "index" is correct - it simplifies navigation.

Positive Changes

Excellent reorganization around user goals rather than API surface
Concise, focused examples replace verbose explanations
Practical patterns for common use cases (guardrails, caching, state management)
Consistent terminology with current ADK-TS API
Proper use of AgentBuilder patterns throughout
Good separation of concerns between the three documentation files

Recommendations

  1. Fix the model name from gemini-2.5-flash to gemini-1.5-flash throughout
  2. Add null safety to llmRequest.contents access
  3. Fix systemInstruction handling to properly handle both string and Content object types
  4. Update the Mermaid diagram to show the iterative nature of model/tool calls
  5. Verify all imports are necessary and correct

The documentation structure and writing quality are excellent - these are primarily technical accuracy fixes needed to ensure the examples work correctly.


…st import

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Timonwa

Timonwa commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

1 and 6 are irrelevant. I have fixed the rest.

@Adebesin-Cell Adebesin-Cell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good 👍

@Timonwa
Timonwa merged commit f974146 into main May 6, 2026
4 checks passed
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.

Review and test the ADK-TS Callbacks documentation

2 participants