Skip to content

feat(ux): fix a few bugs and improve ux#13

Merged
shiqian-su merged 5 commits intomainfrom
dev-ux
Aug 19, 2025
Merged

feat(ux): fix a few bugs and improve ux#13
shiqian-su merged 5 commits intomainfrom
dev-ux

Conversation

@ntudy
Copy link
Contributor

@ntudy ntudy commented Aug 17, 2025

Describe this PR

Checklist for PR

Must Do

  • Write a good PR title and description, i.e. feat(agent): add pdf tool via mcp, perf: make llm client async and fix(utils): load custom config via importlib etc. CI job check-pr-title enforces Angular commit message format to PR title.
  • Run make precommit locally. CI job lint enforce ruff default format/lint rules on all new codes.
  • Run make pytest. Check test summary (located at report.html) and coverage report (located at htmlcov/index.html) on new codes.

Nice To Have

  • (Optional) Write/update tests under /tests for feat and test PR.
  • (Optional) Write/update docs under /docs for docs and ci PR.

Copy link
Contributor

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 improves user experience by fixing proxy configuration logic and providing default base URLs for API endpoints. The changes aim to make the setup process smoother and more intuitive for users.

  • Refactored proxy configuration check in Claude Anthropic client to use a more concise approach
  • Added default base URLs for Anthropic, OpenAI, and OpenRouter APIs in the environment template
  • Reorganized and improved documentation for setting up environment variables

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
libs/miroflow/src/miroflow/llm/providers/claude_anthropic_client.py Simplified proxy configuration logic using walrus operator
apps/run-agent/.env.template Added default base URLs for API endpoints
README.md Restructured environment setup documentation for better clarity

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

http_client_args["headers"] = {"trace-id": trace_id}
if not OmegaConf.is_missing(config, "https_proxy"):
http_client_args["proxy"] = config.env.https_proxy
if (proxy := config.get("env.https_proxy", "???")) != "???":
Copy link

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

Using '???' as a sentinel value is unconventional and unclear. Consider using None as the default value and checking for truthiness: if proxy := config.get("env.https_proxy"):

Suggested change
if (proxy := config.get("env.https_proxy", "???")) != "???":
if proxy := config.get("env.https_proxy"):

Copilot uses AI. Check for mistakes.
http_client_args["headers"] = {"trace-id": trace_id}
if not OmegaConf.is_missing(config, "https_proxy"):
http_client_args["proxy"] = config.env.https_proxy
if (proxy := config.get("env.https_proxy", "???")) != "???":
Copy link

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

[nitpick] The magic string '???' makes the code less readable. The original OmegaConf.is_missing() approach was more explicit about checking for missing configuration values.

Suggested change
if (proxy := config.get("env.https_proxy", "???")) != "???":
if not OmegaConf.is_missing(config, "env.https_proxy"):
proxy = config.env.https_proxy

Copilot uses AI. Check for mistakes.
@ntudy ntudy requested a review from shiqian-su August 19, 2025 06:43
@shiqian-su shiqian-su merged commit 734af4e into main Aug 19, 2025
8 checks passed
@shiqian-su shiqian-su deleted the dev-ux branch August 19, 2025 06:45
Zhudongsheng75 pushed a commit to open-compass/MiroFlow that referenced this pull request Dec 27, 2025
* update default base url

* update .env readme

* fix https

* lint code

* lint

---------

Co-authored-by: Yue Deng <yue.deng@miromind.com>
BinWang28 added a commit that referenced this pull request Mar 11, 2026
feat(agent): add regex boxed extractor
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.

3 participants