Skip to content

refactor(brain): simplify pass on tracked ops scripts#39

Merged
Gradata merged 1 commit into
mainfrom
refactor/simplify-brain-scripts
Apr 14, 2026
Merged

refactor(brain): simplify pass on tracked ops scripts#39
Gradata merged 1 commit into
mainfrom
refactor/simplify-brain-scripts

Conversation

@Gradata

@Gradata Gradata commented Apr 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • Consolidate duplicated Ollama /api/generate wrapper + SDK sys.path wiring across three scripts into new brain/scripts/_common.py
  • Remove dead code: overwritten defaultdict loop in brain_benchmark._simulate_graduation, no-op Forum.add_disagree and unused field import in mirofish_sim
  • Preserve every CLI signature and public symbol (tests import score_brain, Agent, Post, Forum, _generate, _ollama_generate directly)

Touched

  • brain/scripts/_common.py (new, 2 shared helpers)
  • brain/scripts/ab_test_constitutional.py
  • brain/scripts/brain_benchmark.py
  • brain/scripts/mirofish_sim.py
  • Not touched: cross_validate.py, migrate_tree_paths.py, optimization_runner.py — already tight, no shared patterns worth extracting

Test plan

  • --help on every script still works
  • ab_test_constitutional.py --dry-run prints imports = OK
  • pytest tests/ — 2060 passed, 23 skipped

Generated with Gradata

- Extract shared Ollama /api/generate wrapper + SDK-path wiring into
  brain/scripts/_common.py (only consolidation with >=2 call sites).
- ab_test_constitutional + mirofish_sim: delegate to ollama_generate,
  drop duplicated urllib plumbing.
- brain_benchmark: use ensure_sdk_on_path(); delete dead first-pass
  defaultdict loop in _simulate_graduation (was a no-op overwritten
  by the correct second loop).
- mirofish_sim: remove dead Forum.add_disagree() no-op and the unused
  ``field`` import.

CLI signatures preserved. All 2060 tests still pass.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (4)
  • brain/scripts/_common.py is excluded by !brain/**
  • brain/scripts/ab_test_constitutional.py is excluded by !brain/**
  • brain/scripts/brain_benchmark.py is excluded by !brain/**
  • brain/scripts/mirofish_sim.py is excluded by !brain/**

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2ff3531e-2636-44a1-978f-b00cbc8f78d9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying gradata-dashboard with  Cloudflare Pages  Cloudflare Pages

Latest commit: b30f296
Status:🚫  Build failed.

View logs

@Gradata

Gradata commented Apr 14, 2026

Copy link
Copy Markdown
Owner Author

Cloudflare Pages failure: unrelated to this PR, safe to merge

Root cause: The failing check is for the gradata-dashboard CF Pages project, which builds from cloud/dashboard/. This PR only touches brain/scripts/*.py (4 files, 0 dashboard files), so the build failure cannot be caused by these changes.

Evidence this is pre-existing on main:

Commit CF Pages
23ed26a (main HEAD) failure
c8f64ce failure
77d2eab success
2151b14 failure
7e47ba3 failure
14d7426 failure

Other checks on this PR: CodeRabbit pass, test (3.11/3.12/3.13) pass.

Recommendation: Merge this refactor PR. The gradata-dashboard CF Pages build needs a separate fix on main (likely a build config or env/secret issue in the dashboard project, outside the scope of a scripts refactor).

@Gradata Gradata merged commit d44468b into main Apr 14, 2026
5 of 6 checks passed
Gradata added a commit that referenced this pull request Apr 14, 2026
Conflicts in cloud/app/db.py, routes/brains.py, routes/users.py.

Resolution strategy (preserve main's behavior additions, keep simplify
intent where non-conflicting):

- db.py: accepted main's explicit in_= parameter (required by activity.py,
  rule_patches.py, brains.py callers merged via #44). Kept simplify's
  _raise_db_error for uniform error handling.
- routes/brains.py: kept main's batched lessons+corrections IN-query for
  list_brains (perf) + main's new POST /brains endpoint. Preserved
  simplify's _brain_detail helper, Depends(get_brain_for_request)
  pattern, and _is_demo consolidation.
- routes/users.py: accepted main's version wholesale (adds email,
  _derive_plan, _primary_workspace_id scoped notification updates).
  Simplify's _hydrate_workspaces helper dropped — main's inline form
  carries the required new behavior and this PR is a refactor (no
  behavior changes).

No new behavior introduced by this merge commit.
@Gradata Gradata deleted the refactor/simplify-brain-scripts branch April 15, 2026 07:55
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