Skip to content

fix(optuna): harden literal parsing across both desubstitution paths#7

Open
GrigoryEvko wants to merge 1 commit into
FusionBrainLab:mainfrom
GrigoryEvko:fix/optuna-literal-parsing
Open

fix(optuna): harden literal parsing across both desubstitution paths#7
GrigoryEvko wants to merge 1 commit into
FusionBrainLab:mainfrom
GrigoryEvko:fix/optuna-literal-parsing

Conversation

@GrigoryEvko
Copy link
Copy Markdown

The AST-level _EvalCleaner (add_tuned_comment=False path) checks isinstance(arg, (Constant, List, Tuple, Set, Dict)) to decide whether an eval(...) call wraps a literal that can be inlined. This misses UnaryOp(USub, Constant), so eval(-5) survives in the AST path while the source-level _clean_eval_in_source strips it correctly via ast.literal_eval. The two desubstitution modes silently produced different output for the same input.

Reproducer against current main:

import ast
from gigaevo.programs.stages.optimization.optuna.desubstitution import (
    _EvalCleaner, _clean_eval_in_source,
)

src = "x = eval(-5)"
tree = ast.parse(src); _EvalCleaner().visit(tree); ast.fix_missing_locations(tree)
print(ast.unparse(tree))           # before: 'x = eval(-5)' ;  after: 'x = -5'
print(_clean_eval_in_source(src))  # always: 'x = -5'

Fix: in _EvalCleaner.visit_Call, replace the isinstance check with ast.literal_eval(arg) as a try/except predicate so both paths share the same literal-detection logic. Verified across 12 sample inputs — the AST and source paths now produce identical output. Also broaden the (ValueError, SyntaxError) catches around literal_eval in _coerce_param_value and _clean_eval_in_source to also catch MemoryError, RecursionError, and TypeError.

The AST-level `_EvalCleaner` checked `isinstance(arg, (Constant, List, Tuple, Set, Dict))` to decide whether an `eval(...)` argument is a literal. This missed `UnaryOp(USub, Constant)`, so `eval(-5)` survived in the AST path while the source-level `_clean_eval_in_source` stripped it correctly. Switch to `ast.literal_eval` as the predicate so both paths share the same logic.

Also broaden the `(ValueError, SyntaxError)` except clauses around `literal_eval` in `_coerce_param_value` and `_clean_eval_in_source` — it can also raise `MemoryError`, `RecursionError`, and `TypeError` on pathological inputs.

Verified against the source-level path on 12 sample inputs; both produce identical output post-fix.
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.

1 participant