Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Oct 7, 2025

PR Type

Enhancement, Documentation


Description

  • Refine logging levels and formats

  • Add quotes around dynamic log values

  • Minor variable naming cleanup


Diagram Walkthrough

flowchart LR
  UDR["unused_definition_remover.py"] -- "info -> debug; var rename" --> LOG["Logging behavior refined"]
  FO["function_optimizer.py"] -- "quoted values in logs" --> LOG
Loading

File Walkthrough

Relevant files
Enhancement
unused_definition_remover.py
Cleanup unused var and reduce revert log verbosity             

codeflash/context/unused_definition_remover.py

  • Rename unused variable to _method_name.
  • Lower log level from info to debug for revert message.
+2/-2     
Documentation
function_optimizer.py
Standardize quoted values and headings in logs                     

codeflash/optimization/function_optimizer.py

  • Quote function name in progress message.
  • Quote loop count and runtime in info log.
  • Adjust log prefix from h2 to h3.
+3/-3     

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

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

Unused Variable

The renamed variable _method_name is assigned but never used; consider omitting the assignment or using tuple unpacking with _ to avoid linter warnings.

# Find class methods and add their containing classes and dunder methods
for qualified_name in list(self.qualified_function_names):
    if "." in qualified_name:
        class_name, _method_name = qualified_name.split(".", 1)

        # Add the class itself
        expanded.add(class_name)
Log Level Change

The revert action log was downgraded from info to debug; validate that operational visibility isn't lost where revert actions are important for auditing.

    logger.debug("everything done, exiting")
    break

try:
    candidate_index += 1
    get_run_tmp_file(Path(f"test_return_values_{candidate_index}.bin")).unlink(missing_ok=True)
    get_run_tmp_file(Path(f"test_return_values_{candidate_index}.sqlite")).unlink(missing_ok=True)
    logger.info(f"h3|Optimization candidate {candidate_index}/{processor.candidate_len}:")
    code_print(candidate.source_code.flat, file_name=f"candidate_{candidate_index}.py")
    # map ast normalized code to diff len, unnormalized code
    # map opt id to the shortest unnormalized code
    try:
        did_update = self.replace_function_and_helpers_with_optimized_code(
Log Formatting

The metric prefix changed from 'h2|' to 'h3|' and values are now quoted; ensure any downstream log parsers/dashboards expect this schema to avoid broken formatting.

loop_count = max([int(result.loop_index) for result in benchmarking_results.test_results])
logger.info(
    f"h3|⌚ Original code summed runtime measured over '{loop_count}' loop{'s' if loop_count > 1 else ''}: "
    f"'{humanize_runtime(total_timing)}' per full loop"
)
console.rule()
logger.debug(f"Total original code runtime (ns): {total_timing}")

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Unquote numeric and duration values

Do not add quotes around numeric counts and humanized durations in logs; it impairs
readability and may break downstream parsing that expects numbers. Log the values
directly without quoting.

codeflash/optimization/function_optimizer.py [660-661]

-f"h3|⌚ Original code summed runtime measured over '{loop_count}' loop{'s' if loop_count > 1 else ''}: "
-f"'{humanize_runtime(total_timing)}' per full loop"
+f"h3|⌚ Original code summed runtime measured over {loop_count} loop{'s' if loop_count > 1 else ''}: "
+f"{humanize_runtime(total_timing)} per full loop"
Suggestion importance[1-10]: 7

__

Why: The PR added quotes around loop_count and humanize_runtime(total_timing) in a log line; removing them improves readability and avoids potential parsing issues. The suggested change matches the new hunk lines and cleanly reverts the quoting without altering message structure.

Medium
Remove unnecessary quotes in message

Avoid wrapping dynamic values in quotes in user-facing progress messages; it adds
noise and can look unintended. Use the raw value to keep output clean and consistent
with other messages.

codeflash/optimization/function_optimizer.py [307]

-f"Generating new tests and optimizations for function '{self.function_to_optimize.function_name}'",
+f"Generating new tests and optimizations for function {self.function_to_optimize.function_name}",
Suggestion importance[1-10]: 6

__

Why: The existing code adds single quotes around self.function_to_optimize.function_name, which is a minor UX inconsistency; removing them improves readability and matches prior style. The mapping to the new hunk line is correct and the improved_code accurately reflects the change.

Low
Use conventional unused placeholder

If the method name is intentionally unused, replace the variable with '_' to signal
this more clearly and avoid confusion. This keeps the intent obvious and satisfies
linters without introducing misleading pseudo-names.

codeflash/context/unused_definition_remover.py [282]

-class_name, _method_name = qualified_name.split(".", 1)
+class_name, _ = qualified_name.split(".", 1)
Suggestion importance[1-10]: 4

__

Why: Changing _method_name to _ is a minor stylistic tweak; the current name is already prefixed with _ and acceptable. The line reference is correct, but the impact is low and purely stylistic.

Low

@mohammedahmed18 mohammedahmed18 merged commit 1d93fd0 into main Oct 7, 2025
19 of 21 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