refactor: Code quality fix #700
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kingshuk-Microsoft
requested review from
Avijit-Microsoft,
Fr4nc3,
Prajwal-Microsoft,
Roopan-Microsoft,
Vinay-Microsoft,
aniaroramsft,
dgp10801,
marktayl1,
nchandhi and
toherman-msft
Contributor
There was a problem hiding this comment.
Pull request overview
This PR focuses on code-quality cleanups and robustness improvements across backend services, utility scripts, frontend components, and test suites—primarily by tightening exception handling, improving logging/observability, and removing unused imports/variables.
Changes:
- Improved exception handling/logging in backend orchestrator and Cosmos service to avoid silent failures and increase observability.
- Cleaned up unused imports/variables across docs/scripts/frontend to reduce clutter and potential confusion.
- Refined E2E/test logic for better reliability and readability (e.g., clearer branching, improved section removal selection).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/generate_architecture_png.py | Removes unused locals and replaces bare except with specific exception handling for font loading. |
| docs/generate_architecture.py | Removes unused Azure network diagram import to simplify dependencies. |
| content-gen/tests/test_agents.py | Removes unused test imports to reduce noise in the unit test module. |
| content-gen/tests/rai_testing.py | Cleans up imports/flags and adjusts report output path printing (but introduces an auth dependency regression—see PR comments). |
| content-gen/src/backend/services/cosmos_service.py | Adds warning logging instead of silently swallowing cross-partition query failures. |
| content-gen/src/backend/orchestrator.py | Adds debug logging around best-effort JSON parsing and improves observability when compliance parsing fails. |
| content-gen/src/backend/app.py | Removes unused inline asyncio imports in request handlers. |
| content-gen/scripts/test_content_generation.py | Removes unused json import. |
| content-gen/scripts/sample_content_generation.py | Removes unused os import and avoids unused assignment when awaiting generation. |
| content-gen/scripts/post_deploy.py | Removes unused imports and tightens typing imports. |
| content-gen/scripts/create_image_search_index.py | Removes unused json import. |
| archive-doc-gen/tests/e2e-test/tests/test_st_docgen_tc.py | Refactors E2E test logic (exception specificity, section selection logic, logging/branch tweaks). |
| archive-doc-gen/tests/e2e-test/pages/draftPage.py | Removes unused os import. |
| archive-doc-gen/src/frontend/src/pages/chat/Chat.tsx | Removes unused Fluent UI imports to simplify the component import surface. |
| archive-doc-gen/src/frontend/src/components/ChatHistory/chatHistoryListItem.test.tsx | Adjusts rename test mocking/formatting (but leaves a test intent mismatch—see PR comments). |
| archive-doc-gen/src/frontend/src/components/ChatHistory/ChatHistoryListItem.tsx | Removes unused Fluent UI tooltip styles import. |
| archive-doc-gen/src/backend/settings.py | Attempts to ensure parent initializer execution for datasource settings (but introduces duplicate __init__ issue—see PR comments). |
Comments suppressed due to low confidence (1)
archive-doc-gen/src/backend/settings.py:262
- This class does not call DatasourcePayloadConstructor.init during initialization. (_AzureSearchSettings.init may be missing a call to a base class init)
This class does not call DatasourcePayloadConstructor.init during initialization. (_AzureSearchSettings.init may be missing a call to a base class init)
class _AzureSearchSettings(BaseSettings, DatasourcePayloadConstructor):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
archive-doc-gen/src/frontend/src/components/ChatHistory/chatHistoryListItem.test.tsx
Show resolved
Hide resolved
…Azure identity import handling
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request includes a variety of improvements and cleanups across the backend, frontend, and test codebases. The most significant changes focus on improving error handling and logging, refactoring exception usage, cleaning up unused imports, and making tests more robust and readable.
Backend Improvements:
orchestrator.pyandcosmos_service.py, by catching specific exceptions, logging debug or warning messages, and avoiding silent failures. This increases observability and maintainability. [1] [2] [3] [4]asyncioandjson) from several backend scripts and modules, reducing clutter and potential confusion. [1] [2] [3] [4] [5] [6]Frontend and TypeScript Updates:
Testing and E2E Test Improvements:
Exception) instead of using bareexcept, improving error clarity and debugging. [1] [2] [3] [4]Miscellaneous Cleanups:
_AzureSearchSettingsto ensure proper initialization of parent classes.sample_content_generation.pyto avoid unused assignment.These changes collectively improve code quality, maintainability, and robustness across the project.
Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
GP Testing