Skip to content

Improving readability of scenario 7 #25

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

Merged
merged 3 commits into from
Jun 23, 2025
Merged

Improving readability of scenario 7 #25

merged 3 commits into from
Jun 23, 2025

Conversation

aaronpowell
Copy link
Member

  • Moving to slnx from sln
  • Updated docs
  • Simplification of the agent codebase to improve discoverability and support running locally

…upport running locally

* The changes introduce a new agent-based architecture for insights generation using Microsoft Semantic Kernel, with dedicated agent classes for sentiment and language detection, and a new extension method for registering these agents and the generator in the DI container.
* The Generator class is refactored to use dependency injection for agents and logging, removing direct agent instantiation and improving orchestration.
* The OpenAI client and embedding generator setup in both Insights and Products projects is modernized to use Azure Identity and configuration-based deployment names, supporting better cloud integration and local development.
* The memory context in Products is refactored to use new AI interfaces, with improved prompt handling and logging.
* Endpoint mapping in Insights is updated to use static lambdas and typed results for clarity and performance.
* The solution file now includes the README.md as a solution item.
* The README.md is updated to clarify configuration steps, correct project names, and provide improved instructions for setting up Azure OpenAI connections and telemetry.
* OpenTelemetry configuration is enhanced to include additional meters and sources for better observability, including experimental AI metrics.
Copy link

👋 Thanks for contributing @aaronpowell! We will review the pull request and get back to you soon.

Copy link

Check Country Locale in URLs

We have automatically detected added country locale to URLs in your files.
Review and remove country-specific locale from URLs to resolve this issue.

Check the file paths and associated URLs inside them.
For more details, check our Contributing Guide.

File Full Path Issues
scenarios/08-Sql2025/README.md
#LinkLine Number
1https://learn.microsoft.com/en-us/sql/relational-databases/vectors/vectors-sql-server?view=sql-server-ver1767

Copy link

Check Broken URLs

We have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue.

Check the file paths and associated broken URLs inside them.
For more details, check our Contributing Guide.

File Full Path Issues
README.md
#LinkLine Number
1https://dcbadge.vercel.app/api/server/ByRwuEEgH411

@elbruno elbruno requested a review from Copilot June 23, 2025 13:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR standardizes solution files to the new .slnx format, updates documentation links, and refactors the agent codebase (scenario 7) to use dependency injection, primary constructors, and modern Azure OpenAI client patterns for improved local development and discoverability.

  • Converted legacy .sln files to <Solution> XML (.slnx) and removed old .sln clutter
  • Updated README.md pointers to .slnx files across scenarios
  • Simplified Azure OpenAI client configuration and refactored agent/MemoryContext classes to leverage DI and raw string literals

Reviewed Changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scenarios/08-Sql2025/src/eShopLite-Sql2025.slnx Added new XML-based solution file .slnx
scenarios/08-Sql2025/README.md Updated solution link to point at .slnx
scenarios/07-AgentsConcurrent/src/eShopServiceDefaults/Extensions.cs Added telemetry instrumentation for Experimental.Microsoft.Extensions.AI
scenarios/07-AgentsConcurrent/src/eShopAppHost/Program.cs Introduced conditional OpenAI connection logic
scenarios/07-AgentsConcurrent/src/Products/Memory/MemoryContext.cs Refactored class to primary constructor and raw prompt literal
Comments suppressed due to low confidence (3)

scenarios/07-AgentsConcurrent/src/eShopAppHost/Program.cs:54

  • The variable aoai is not defined in this scope, which will cause a compilation error. Replace aoai with the correct reference returned by your AddAzureOpenAIClient call or declare it earlier.
    openai = aoai;

scenarios/07-AgentsConcurrent/src/Products/Memory/MemoryContext.cs:17

  • [nitpick] Raw string literals preserve leading whitespace in each line; the indentation here will be sent to the model. Consider aligning the delimiters or trimming indentation so the prompt is formatted cleanly.
    private const string _systemPrompt = """

scenarios/07-AgentsConcurrent/src/eShopServiceDefaults/Extensions.cs:74

  • [nitpick] Confirm that the meter/source name matches the actual library's SDK namespace. If the instrumentation targets Microsoft.Extensions.AI, update the meter name accordingly to ensure telemetry is captured.
                        .AddMeter("Experimental.Microsoft.Extensions.AI");

@elbruno elbruno marked this pull request as ready for review June 23, 2025 13:55
@elbruno elbruno merged commit 9756dff into main Jun 23, 2025
8 of 10 checks passed
@elbruno elbruno deleted the agent-code-cleanup branch June 23, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants