Skip to content

[FirebaseAI] Fix minor issues with reference docs #1251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

a-maurice
Copy link
Collaborator

@a-maurice a-maurice commented May 9, 2025

Description

Provide details of the change, and generalize the change in the PR title above.

Change properties to not use lambda notation, since the generated ref docs weren't handling them well. Add comments to ResponseModality enum.


Testing

Describe how you've tested these changes.

Looking at the generated ref docs


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Copy link

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

Choose a reason for hiding this comment

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

Hello @a-maurice, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request focuses on improving the generated reference documentation for the FirebaseAI library. It addresses two main issues: hiding the internal namespace from the generated documentation and modifying properties to use explicit getter syntax instead of lambda notation, which was causing problems with the documentation generation process. The changes involve updates to the CMake build configuration to exclude internal files from documentation generation and modifications to several C# files to change property definitions.

Highlights

  • Documentation Generation: The primary goal is to improve the quality and accuracy of the generated reference documentation for the FirebaseAI library.
  • Internal Namespace Hiding: The pull request ensures that the Internal namespace is excluded from the generated documentation, preventing internal implementation details from being exposed to users.
  • Property Syntax Modification: Properties are changed to use explicit getter syntax instead of lambda notation to ensure correct documentation generation.

Changelog

Click here to see the changelog
  • firebaseai/CMakeLists.txt
    • Added a filter to exclude files in the src/Internal/ directory from the documentation generation process.
  • firebaseai/src/Candidate.cs
    • Modified the SafetyRatings property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/Chat.cs
    • Modified the History property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/Citation.cs
    • Modified the Citations property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/CountTokensResponse.cs
    • Modified the PromptTokensDetails property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/FirebaseAI.cs
    • Modified the DefaultInstance property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/GenerateContentResponse.cs
    • Modified the Candidates property to use explicit getter syntax instead of lambda notation.
    • Modified the SafetyRatings property to use explicit getter syntax instead of lambda notation.
    • Modified the PromptTokensDetails property to use explicit getter syntax instead of lambda notation.
    • Modified the CandidatesTokensDetails property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/LiveSessionResponse.cs
    • Modified the FunctionCalls property to use explicit getter syntax instead of lambda notation.
    • Modified the FunctionIds property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/ModelContent.cs
    • Modified the Role property to use explicit getter syntax instead of lambda notation.
    • Modified the Parts property to use explicit getter syntax instead of lambda notation.
  • firebaseai/src/Schema.cs
    • Modified the EnumValues property to use explicit getter syntax instead of lambda notation.
    • Modified the RequiredProperties property to use explicit getter syntax instead of lambda notation.
    • Modified the PropertyOrdering property to use explicit getter syntax instead of lambda notation.
    • Modified the AnyOfSchemas property to use explicit getter syntax instead of lambda notation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


A doc's clarity,
A guide for all to see,
Code's best friend, true.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

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

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 addresses issues with reference documentation generation by changing C# properties from lambda notation (expression-bodied members) to standard get-accessor properties. Additionally, it hides the Internal namespace from the generated documentation by updating the CMakeLists.txt file.

The changes are clear, directly address the stated problem, and the reasoning for moving away from expression-bodied members (tooling compatibility for docs) is well-understood. The update to CMakeLists.txt effectively filters out internal implementation details from the public API documentation.

One minor point regarding process: the "Testing" section in the PR description is still a placeholder. It would be beneficial to describe how these changes were verified, presumably by generating the reference documentation and checking for the corrected property rendering and exclusion of internal members.

Summary of Findings

  • Stylistic Consistency in Property Getters: There's a minor stylistic inconsistency in how simple getters are formatted. Some simple getters (e.g., in Schema.cs) use a compact single-line format like get { return _value; }, while others with similarly simple logic (e.g., FirebaseAI.DefaultInstance) use a multi-line format. While both are valid, adopting a consistent style for simple one-line getters could improve uniformity. This is a low-severity observation and not commented on directly due to review settings.
  • Minor Formatting in Chat.cs: In firebaseai/src/Chat.cs, the History property's getter has an extra leading space before the return statement (return new ReadOnlyCollection<ModelContent>(chatHistory);). This is a very minor formatting detail. This is a low-severity observation and not commented on directly due to review settings.

Merge Readiness

The changes in this pull request are well-justified and correctly implemented to fix issues with reference documentation generation. There are no critical or high-severity issues identified. The minor stylistic points noted in the findings summary are of low severity and do not block merging.

Based on this review, the code appears to be in good shape to be merged once the testing details are filled in, if required by your process. As a reviewer, I am not authorized to approve the pull request, so please ensure it undergoes any further necessary reviews and approvals before merging.

@a-maurice a-maurice requested a review from cynthiajoan May 10, 2025 00:54
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.

1 participant