Skip to content

skydiscover/extras/evolve_analyzer - #73

Open
oprince wants to merge 26 commits into
skydiscover-ai:mainfrom
oprince:postmortem-analysis
Open

skydiscover/extras/evolve_analyzer#73
oprince wants to merge 26 commits into
skydiscover-ai:mainfrom
oprince:postmortem-analysis

Conversation

@oprince

@oprince oprince commented May 13, 2026

Copy link
Copy Markdown

post-mortem diagnostic tool for evolutionary optimization runs directly into skydiscover as an optional sub-package, following the existing extras/ pattern (alongside monitor/ and external/).

  • Adds skydiscover/extras/evolve_analyzer/ with 26 source files: ingestion, quantitative (11 analyzers), qualitative (6 LLM judges), vendored llm/ layer, coordinator, report_synthesizer, dashboard, historical_db, and bundled config/
  • Adds tests/extras/evolve_analyzer/ with 303 passing tests
  • Adds [evolve-analyzer] optional dependency group to pyproject.toml
  • Adds run-evolve-analysis and dashboard-evolve-analysis CLI entry points
  • Fixes config file path in coordinator.py (parents[2] → parent) to match the new bundled layout

Details and samples can be found here

Install with: pip install skydiscover[evolve-analyzer]

Integrates the standalone post-mortem diagnostic tool for evolutionary
optimization runs directly into skydiscover as an optional sub-package,
following the existing extras/ pattern (alongside monitor/ and external/).

- Adds skydiscover/extras/evolve_analyzer/ with 26 source files:
  ingestion, quantitative (11 analyzers), qualitative (6 LLM judges),
  vendored llm/ layer, coordinator, report_synthesizer, dashboard,
  historical_db, and bundled config/
- Adds tests/extras/evolve_analyzer/ with 303 passing tests
- Adds [evolve-analyzer] optional dependency group to pyproject.toml
- Adds run-evolve-analysis and dashboard-evolve-analysis CLI entry points
- Fixes config file path in coordinator.py (parents[2] → parent) to
  match the new bundled layout

Install with: pip install skydiscover[evolve-analyzer]

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@oprince
oprince marked this pull request as draft May 13, 2026 10:45

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new evolve-analyzer package, providing a suite of tools for post-mortem analysis of evolutionary code optimization experiments. The changes include CLI entry points for running analyses and viewing reports, a dashboard for visualizing results, and a comprehensive set of quantitative and qualitative analyzers (including LLM-based judges). My review identified several areas for improvement, primarily regarding resource management, error handling, and adherence to Python best practices, such as avoiding library-level sys.exit() calls and ensuring proper resource cleanup in database connections.

Comment thread skydiscover/extras/evolve_analyzer/coordinator.py
Comment thread skydiscover/extras/evolve_analyzer/historical_db.py Outdated
Comment thread skydiscover/extras/evolve_analyzer/cli.py Outdated
Comment thread skydiscover/extras/evolve_analyzer/cli.py Outdated
Comment thread skydiscover/extras/evolve_analyzer/dashboard.py Outdated
Comment thread skydiscover/extras/evolve_analyzer/llm/client.py
oprince and others added 13 commits May 13, 2026 15:17
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace sys.exit() with click.ClickException in CLI handlers
- Fix _connect() resource leak via contextmanager (commit/rollback/close)
- Move inline `import math` to top-level imports
- Replace deprecated subprocess.call with subprocess.run
- Narrow bare except Exception clauses to specific exception types
- Reuse running event loop in _get_or_create_event_loop instead of always creating new

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collapse stagnation period alerts with the same severity, failure type,
recommendation, and LLM analysis into a single expander showing all
iteration ranges, instead of repeating one entry per occurrence.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Detect algorithm class (population_evolutionary / serial_refinement /
  bayesian_optimization) and algorithm name from ingestion source
- Apply per-class rating thresholds for regression frequency and
  exploration diversity so scores reflect algorithm-specific expectations
- Restructure report: merge LLM judge into "Experiment Analysis Parameters"
  section; add new "Experiment Setup" section with algorithm name/class
  and island count
- Add EvolveLoopReport fields: algorithm_class, algorithm_name, num_islands
- Add test coverage for detect_algorithm_class

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests under tests/extras/evolve_analyzer/ import pandas at module level,
which is only available via --extra evolve-analyzer. Without it, pytest
fails to collect 6 test files (exit code 2).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add _parse_eval_error_records fallback: when _parse_skydiscover_log
  finds no iteration records, synthesize failed iterations from
  "Failed to extract Go code" and "Build failed" lines, grouping
  events within a 120s window per iteration
- Detect algorithm name from log filename ({algo}_{YYYYMMDD}_{HHMMSS}.log)
  when no JSONL stats file exists (e.g. gepa runs)
- Map "failed"/"succeeded" in _STATUS_MAP; check outcome field as
  fallback in _fill_derived_fields so log-parsed records are no
  longer silently marked evaluation_status=success
- Escalate crash/format_invalid dominant stagnation streaks to
  critical severity (same logic as identical_output)
- Show crash details in report for non-critical crash-dominant
  stagnation periods, not only for critical ones
- Override executive summary with a clear failure message when
  n_successful == 0 instead of showing a misleading averaged rating

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gepa_native logs use "Iteration N: REJECTED/ACCEPTED child (child_score=X
<= parent_score=Y)" and "Iteration N: Program UUID (parent: UUID) completed
in Xs" formats, neither of which matched the existing "Iteration N failed:"
pattern. This caused the parser to emit only 1 record (the trailing
pending_success flush) instead of all iterations.

Add _LOG_ITER_GEPA_OUTCOME and _LOG_ITER_PROG_COMPLETED regexes and
corresponding handlers in _parse_skydiscover_log. Also fix adapt_skydiscover
to use setdefault so pre-populated parent_score/score_delta are not
overwritten, and track prev_score as a running max so rejected candidates
with negative scores don't corrupt the parent_score of subsequent iterations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- --source skydiscover now detects openevolve_*.log files and delegates
  to a new _parse_openevolve_log_full() parser instead of the skydiscover
  log parser, which missed all iterations for these runs
- _parse_openevolve_log_full extracts iteration records from the parallel
  controller's two-line format (completed + Metrics) and captures island_id
  via MAP-Elites lines; parent propagation fills 100% of island coverage
- --source openevolve also gains log-based fallback when no
  evolution_trace.jsonl is present (previously fell back to checkpoints only)
- No logic duplication: adapt_skydiscover reuses the same parser

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…opulation algorithms

In multi-island runs, short stagnation streaks (length 1-4) are expected
statistical noise from island interleaving and inflate the evidence section
without affecting the rating. Only alert-level periods (length >= threshold)
are now listed, with a trailing count of suppressed non-alert streaks.

Island IDs are shown per alert period when num_islands > 1, derived from
the new island_id field on IterationSummary.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sentinel evaluator failures (score=-100000) were forcing the y-axis to
span the full -100k range, making the actual score progression (-822 to
+8.826) invisible as a flat line near zero.

Clip y-axis to the 2nd-100th percentile range with 10% padding using
autorange=False. Anchor the worst-score annotation to the bottom of the
visible area when the worst point falls outside the clipped range.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extends the tool-aware rating system to cover the convergence dimension,
completing the full set of three dimensions called out in the enhancement
plan (regression frequency, exploration diversity, convergence rate).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n to exploration

Params like island_id are structural algorithm identifiers, not tunable
hyperparameters — showing them as bound-hit is misleading. The evidence
line now filters them out (shows 'none' when island_id is the only hit).

Island concentration info is preserved: dominant_island_id is recorded in
SearchSpaceMetrics and surfaced in the Exploration dimension's evidence
and recommendation instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@oprince
oprince marked this pull request as ready for review June 2, 2026 12:05
oprince and others added 3 commits June 2, 2026 15:15
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@oprince

oprince commented Jun 2, 2026

Copy link
Copy Markdown
Author

Hi @lynnliu030 and @shubham3-ucb 👋

Would you have time to review this PR? It adds an optional evolve-analyzer sub-package under extras/, following the existing pattern alongside monitor/ and external/.

It's a self-contained post-mortem diagnostic tool for evolutionary optimization runs — quantitative analyzers, LLM-based qualitative judges, a Streamlit dashboard, and 303 passing tests. The main touchpoints with the existing codebase are pyproject.toml (new optional dependency group) and the CI workflow.

Happy to walk through anything or break the review into sections if that's easier given the size. No rush — whenever you get a chance works great.

Thanks!
Orit

@shubham3-ucb

Copy link
Copy Markdown
Collaborator

@oprince this is a self-contained, opt-in subsystem and the architecture is reasonable. It is isolated from core skydiscover and the 355 tests pass. I reviewed it for crash-safety and data integrity and ran each item below, so the repros are copy-pasteable. These affect the correctness of the output, so I think they are worth addressing before the merge.

Data correctness (these change reported results):

  1. _extract_score treats a real 0.0 as missing. _extract_score({"combined_score": 0.0, "score": 5.0}) returns 5.0. A run that legitimately scores 0.0 gets the wrong score and delta. If 0.0 is a valid score, the x or y selection should be x if x is not None else y.
  2. The same function drops a top-level score when a metrics dict is present: _extract_score({"metrics": {"foo": 1}, "score": 3.0}) returns None.
  3. _normalise_status maps anything unknown to success: _normalise_status("weird") and _normalise_status(None) both return "success", so an unrecognized failure counts toward the success rate.
  4. analyze_convergence: time_to_best_fraction uses the iteration value rather than the index, so it can leave [0, 1]. Records whose iteration starts at 100 give time_to_best_fraction = 20.8. The same function also raises KeyError on a record missing iteration (the change_points line subscripts ["iteration"] while the rest of the function uses .get).
  5. LLM cache key has no model identity. The call site is cache_call(client.invoke, self.cache_dir, prompt), so the same prompt against two different models returns the first model's cached answer. The module docstring says it keys on "prompt + model identity."
  6. analyze_exploration uses random.sample with no seed, so structural_diversity_index changes run to run on identical input.
  7. The GEPA log parser truncates scientific-notation scores: child_score=1.5e-3 is captured as 1.5 (the score group is [-\d.]+, which excludes e).

Safety / crashes:

  1. dashboard.py:1340 runs eval() on an evidence string. eval("__import__('os').getpid()") executes; eval("foo") raises an uncaught NameError (the except only catches ValueError, SyntaxError). The same file uses ast.literal_eval for the analogous sentinel case lower down, which is the safe form.
  2. _stars(3.5) raises TypeError ("●" * float). A non-integer rating crashes the panel (_stars(3), _stars(None) are fine).
  3. Crash-sample rendering hard-subscripts s["error"] / s["iteration"]; a sample missing either key raises KeyError.

Minor: historical_db interpolates metric_name and filter keys as SQL identifiers (values are parametrized). It is fine today since callers pass literals, but worth validating against the known column set. CI is red only on black (23 files), so black skydiscover/ clears it.

Scope note: I verified crash-safety and the data-path items above by running them. I did not independently validate the statistical formulas in the analyzers or run the full pipeline end to end, so this is not a sign-off on numerical correctness.

oprince and others added 3 commits June 7, 2026 09:58
- Fix _extract_score treating 0.0 as missing (use None checks instead of falsy or-chains)
- Fix _extract_score dropping top-level score when metrics dict is present but empty
- Map unknown evaluation statuses to "unknown" instead of "success"
- Fix time_to_best_fraction exceeding [0,1] by using index-based fraction
- Fix KeyError on missing iteration key in convergence change_points
- Include model identity in LLM cache key to prevent cross-model collisions
- Use seeded RNG for deterministic structural_diversity_index sampling
- Fix GEPA log parser regex to handle scientific notation scores
- Replace eval() with ast.literal_eval() for evidence string parsing
- Fix _stars() TypeError when rating is a float
- Use .get() for crash-sample rendering to avoid KeyError on missing keys

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When exploration rating is poor (≤2), append specific suggested actions
based on run metrics (exploit fraction, cluster count, revert frequency)
instead of only the generic "add restart or escape mechanisms" advice.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@oprince

oprince commented Jun 11, 2026

Copy link
Copy Markdown
Author

@lynnliu030 @shubham3-ucb, would you like to have a live review session?

oprince and others added 2 commits June 15, 2026 13:05
The log parser missed solution-evolution iterations in evox runs because:
1. _LOG_ITER_SUCCESS regex required combined_score= immediately after the
   colon, but kernel evaluators emit speedup= first
2. In co-evolution logs, "Iteration N failed:" from search-strategy
   meta-evolution was captured while "Iteration N: Program X completed"
   from solution evolution was ignored

Fix: allow combined_score= anywhere in the metrics string, parse
"Iteration N: ... completed" lines as primary iteration signals, and
deduplicate so completed records override failed meta-evolution records
for the same iteration number.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Score Progression chart: replace percentile-based y-clipping with IQR
fences so sentinel scores (-1e6) don't collapse the visible range.
Add plateau onset vertical marker from Convergence/Ceiling evidence.

Ceiling analyzer: flat_trend_start now requires meaningful improvement
(≥0.5% of total score range) to reset, and estimated_gain_probability
now measures frontier advancement (best_so_far advanced) instead of
parent-beating (score_delta > 0).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
oprince and others added 4 commits June 15, 2026 15:19
… thresholds

The regression dimension summary text showed default ranges (e.g. "15–30%")
even when algorithm-specific thresholds were active, making the summary
contradict its own evidence line. Now both the summary and the note use the
actual thresholds from the rating context.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Island count: use current_island_idx instead of always taking first
  island's idx (was always reporting 1 island)
- Sub-metric analyzer: exclude negative-score records so sentinel/failed
  eval metrics (runtime=inf, speedup from incorrect kernels) don't appear
  as "best" values
- Stagnation detector: skip negative-score iterations so infrastructure
  failures don't inflate stagnation counts
- Regression analyzer: filter out failed evals and recompute deltas
  between valid adjacent records to prevent artificial severe regressions
- Final score: use cumulative max (running best at end) instead of last
  child_score which may be a regression
- Efficiency analyzer: handle None/negative scores as NaN to prevent
  corruption of best_so_far curve and initial_score
- Infrastructure analyzer: use median of first N healthy records as
  baseline instead of p25 of last N (which inflated spike ratios on
  small/noisy samples)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Evox co-evolution runs have a secondary LLM that evolves the search
strategy itself. When that LLM is unauthorized (401), all iterations
silently fall back to the initial strategy — causing the exploration
and stagnation issues the report already flags, but without surfacing
the root cause.

Changes:
- Add _extract_meta_evo_signals() to parse search/ directory for
  fallback rates, failed models, and error types
- Extend _extract_infra_log_signals() to detect 401 auth errors
- Fix missing signal injection in the log-only ingestion path
- Replace "sentinel" jargon with clear descriptions in report text
- Surface meta-evolution health in Infrastructure dimension with
  appropriate rating downgrades (fully broken: -2, mostly broken: -1)
- Always show meta-evolution status in evidence when data exists

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

2 participants