Skip to content

Conversation

@HeshamHM28
Copy link
Contributor

@HeshamHM28 HeshamHM28 commented Aug 15, 2025

PR Type

Enhancement


Description

  • Handle create_staging response status codes

  • Print staging URL on success

  • Print failure message with status code

  • Generate URL using trace and experiment ID


Diagram Walkthrough

flowchart LR
  A["Create staging review"] --> B["Check response status"]
  B -- "200 OK" --> C["Print success with link"]
  B -- "Other status" --> D["Print failure message"]
Loading

File Walkthrough

Relevant files
Enhancement
function_optimizer.py
Add staging link logging                                                                 

codeflash/optimization/function_optimizer.py

  • Capture create_staging response in response
  • Check response.status_code for success
  • Print staging URL on status 200
  • Print error message on non-200 status
+7/-1     

@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

Undefined variable

The variable exp_type used in URL generation may be undefined in this context, causing a NameError. Ensure it’s properly defined or passed in.

staging_url = f"https://app.codeflash.ai/review-optimizations/{self.function_trace_id[:-4] + exp_type if self.experiment_id else self.function_trace_id}"
URL generation

Hardcoded slicing of function_trace_id and concatenation may result in incorrect or invalid staging URLs. Consider validating the resulting URL or refactoring this logic.

staging_url = f"https://app.codeflash.ai/review-optimizations/{self.function_trace_id[:-4] + exp_type if self.experiment_id else self.function_trace_id}"
console.print(f"[bold green]✅ Staging created:[/bold green] [link={staging_url}]{staging_url}[/link]")
Error logging

On failure, only the status code is printed. Including additional context like the response body or error message will aid debugging.

else:
    console.print(f"[bold red]❌ Failed to create staging [/bold red] — status {response.status_code}")

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use Response.ok for success

Use the Response.ok property to catch any 2xx status instead of checking only for
200. This covers other successful codes like 201. It also reads more clearly.

codeflash/optimization/function_optimizer.py [1251]

-if response.status_code == 200:
+if response.ok:
Suggestion importance[1-10]: 6

__

Why: Checking response.ok covers all 2xx status codes and improves readability over a single 200 check.

Low
Log response details on failure

Include the response body or text in the failure log to aid debugging. It will
provide more context about why staging creation failed.

codeflash/optimization/function_optimizer.py [1255]

-console.print(f"[bold red]❌ Failed to create staging [/bold red] — status {response.status_code}")
+console.print(
+    f"[bold red]❌ Failed to create staging[/bold red] — status {response.status_code}, details: {response.text}"
+)
Suggestion importance[1-10]: 6

__

Why: Including response.text in the error message provides additional context for debugging staging failures.

Low
Extract staging URL from response

Prefer extracting the actual staging URL from the API response if it’s provided in
JSON payload, falling back to manual construction. This ensures the printed link
matches the server’s canonical URL.

codeflash/optimization/function_optimizer.py [1252]

-staging_url = f"https://app.codeflash.ai/review-optimizations/{self.function_trace_id[:-4] + exp_type if self.experiment_id else self.function_trace_id}"
+staging_url = response.json().get(
+    "staging_url",
+    f"https://app.codeflash.ai/review-optimizations/{(self.function_trace_id[:-4] + exp_type) if self.experiment_id else self.function_trace_id}"
+)
Suggestion importance[1-10]: 5

__

Why: Falling back to a manually constructed URL ensures the printed link matches the server-provided canonical URL when available.

Low

aseembits93
aseembits93 previously approved these changes Aug 15, 2025
@HeshamHM28 HeshamHM28 requested a review from aseembits93 August 18, 2025 16:01
Copy link
Contributor

@aseembits93 aseembits93 left a comment

Choose a reason for hiding this comment

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

lgtm

@aseembits93
Copy link
Contributor

screenshot just in case you saved it @HeshamHM28 , Looks good to me

@HeshamHM28 HeshamHM28 enabled auto-merge August 19, 2025 17:05
@HeshamHM28 HeshamHM28 merged commit fbb4bbe into main Aug 19, 2025
18 of 19 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.

3 participants