Skip to content

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Oct 22, 2025

PR Type

Enhancement


Description

  • Add CFWEBAPP_BASE_URL for web app links

  • Use web base URL in staging link

  • Compute trace_id before URL formatting


Diagram Walkthrough

flowchart LR
  A["cfapi.py"] -- "define CFWEBAPP_BASE_URL (local/prod)" --> B["function_optimizer.py"]
  B["function_optimizer.py"] -- "build staging link with CFWEBAPP_BASE_URL" --> C["User gets correct web URL"]
Loading

File Walkthrough

Relevant files
Enhancement
cfapi.py
Add CF Web App base URL constants                                               

codeflash/api/cfapi.py

  • Introduce CFWEBAPP_BASE_URL for local.
  • Set CFWEBAPP_BASE_URL for production.
  • Keep CFAPI_BASE_URL unchanged for API calls.
+2/-0     
function_optimizer.py
Use web base for staging review URLs                                         

codeflash/optimization/function_optimizer.py

  • Import CFWEBAPP_BASE_URL instead of CFAPI_BASE_URL.
  • Build staging URL using web app base.
  • Extract trace_id before formatting URL.
+3/-2     

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The new trace_id computation uses exp_type in the expression, but exp_type is not defined in the shown context. This could raise a NameError at runtime when creating the staging URL. Verify exp_type is defined in scope or adjust the logic.

trace_id = self.function_trace_id[:-4] + exp_type if self.experiment_id else self.function_trace_id
staging_url = f"{CFWEBAPP_BASE_URL}/review-optimizations/{trace_id}"
Config Consistency

CFWEBAPP_BASE_URL is set to the same value as CFAPI_BASE_URL for prod. Confirm that the web app and API share the same host and path structure; otherwise staging URLs may be incorrect in some environments.

    CFWEBAPP_BASE_URL = "http://localhost:3000"
    logger.info(f"Using local CF API at {CFAPI_BASE_URL}.")
    console.rule()
else:
    CFAPI_BASE_URL = "https://app.codeflash.ai"
    CFWEBAPP_BASE_URL = "https://app.codeflash.ai"

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Safeguard trace ID slicing

Guard against short function_trace_id values when slicing to avoid accidental empty
or malformed IDs. Ensure the slice only runs when length permits.

codeflash/optimization/function_optimizer.py [1485-1487]

-trace_id = self.function_trace_id[:-4] + exp_type if self.experiment_id else self.function_trace_id
+if self.experiment_id and len(self.function_trace_id) >= 4:
+    trace_id = self.function_trace_id[:-4] + exp_type
+else:
+    trace_id = self.function_trace_id
 staging_url = f"{CFWEBAPP_BASE_URL}/review-optimizations/{trace_id}"
Suggestion importance[1-10]: 7

__

Why: This is a valid defensive change against short function_trace_id when using [:-4], preventing malformed IDs. It's contextually accurate and improves robustness without altering behavior otherwise.

Medium
General
Clarify selected base URLs

Log which webapp base URL is being used to avoid confusion when links are generated.
Add an info log in the non-local branch and include CFWEBAPP_BASE_URL in the local
log message.

codeflash/api/cfapi.py [29-36]

 if os.environ.get("CODEFLASH_CFAPI_SERVER", "prod").lower() == "local":
     CFAPI_BASE_URL = "http://localhost:3001"
     CFWEBAPP_BASE_URL = "http://localhost:3000"
-    logger.info(f"Using local CF API at {CFAPI_BASE_URL}.")
+    logger.info(f"Using local CF API at {CFAPI_BASE_URL}, webapp at {CFWEBAPP_BASE_URL}.")
     console.rule()
 else:
     CFAPI_BASE_URL = "https://app.codeflash.ai"
     CFWEBAPP_BASE_URL = "https://app.codeflash.ai"
+    logger.info(f"Using CF API at {CFAPI_BASE_URL}, webapp at {CFWEBAPP_BASE_URL}.")
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly targets the new/modified env-based URL selection and adds helpful logging for CFWEBAPP_BASE_URL. It's accurate and improves observability, but it's a minor enhancement.

Low

@misrasaurabh1 misrasaurabh1 merged commit ff91f9c into main Oct 22, 2025
23 of 24 checks passed
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