modularize
Turn one service module (lambda_functions.py, s3.py, ...) into a package of small modules - core (models, analysis, analyzer) that every front-end shares, and front-ends (report blocks, View, explorer
SKILL.md.claude/skills/modularize/SKILL.md
GitHubModularize: one service, one package
Request: $ARGUMENTS. A service name (lambda_functions, s3, _bedrock, ...) means: do that service's step,
following the recipe below. tooling means Phase 0. status means: read the progress table, check it against the
tree (ls src/aws_analyzer/), and say what's next.
Read CLAUDE.md first. Everything there still holds, and this plan doesn't relax any of it, except where it says so explicitly (the per-file lists in "Config files that are keyed by file").
Why
Each service is one file with five # ==== sections, and four of them are 8,500 to 12,000 lines. They grow with
every command, window tab and agent tool. Nobody (human or agent) can hold such a file in their head, and the parts
used by different consumers are mixed together.
The library has one core and several front-ends:
┌──────────── core (per service) ─────────────┐
│ helpers · models · analysis · analyzer │
│ (parses, decides, talks to AWS, returns data)│
└──────▲───────────────▲───────────────▲───────┘
│ │ │
View (reports) explorer / window (widgets) agent toolkit (AI, in agent/)
└──── _kit's UI runtime ────┘ └── agent/'s runtime
The goal: the core is written once and every front-end uses it its own way. A front-end only reads the core. The core never knows a front-end exists. That only holds if the code is laid out so it's visible and checked.
The decisions (already made, don't reopen them in a step's PR)
- Each service becomes a package with the same layer names. Every package uses the same names, so an agent who knows one service knows where things are in all of them. Small services stay flat; big ones get sub-packages where they grow.
- The public import path doesn't change.
from aws_analyzer.lambda_functions import parse_function, LambdaViewkeeps working: the package's__init__.pyre-exports every public name. Submodule paths (aws_analyzer.lambda_functions.analysis.logs) are internal and may move again. - Moves only. No behaviour change. A split step changes no output, no public name, no method or attribute of a
public class. It doesn't fix bugs, rename things or clean up style on the way: moved code is pasted as it was,
so
git diff --color-movedshows it as moved and review is fast. Anything else goes in its own PR. - A module stays under
MAX_MODULE_LINES(1,500).tests/test_style.pyenforces it. The files that were already longer are listed inLONG_FILESuntil they're split. - A class that's too big for one module is split into mixins by concern, in the same commit. For example,
LambdaExplorer(_LogsTab, _CodeTab, ..., _Explorer)orS3View(_ListingCommands, _FileCommands, ..., _BaseView). Methods and attributes keep their names, so tests and users see the same class. Turning mixins into proper components later is a separate design change, not part of this plan. - Agent toolkits stay in
agent/toolkits/. We considered moving each toolkit into its service's package and rejected it. A toolkit subclassesagent.toolkits.base.Toolkit, which needs the agent'sBudget, and a tool can join two services (s3_triggersreads S3 and, throughpeer("lambda"), Lambda). Either would make a service import the agent or another service, which breaks "importing one analyzer loads no other". The agent is a cross-service front-end, and it keeps being the companion of the services it reads. - The agent reads only the core's public API. Today it reaches into private names
(
LambdaAnalyzer._window,._numbers,S3Analyzer._cloudwatch_in,s3._rule_scope,s3._rule_actions,s3._exposure,s3._encryption_label,s3._block_label). Each service's step makes the ones it owns public (a plain rename, all callers updated) as its first commit, because__init__only re-exports public names. _kitfiles don't move.rules.pyclassifies them instead.- Core-safe:
fmt,text,errors,deps,analyzer. - UI:
blocks,render,view,window,progress,highlight,jsontree. - The one exception is
render._call(and_signature). The core uses them to write calls into findings. They stay allowed until Phase 2 removes the need.
- Core-safe:
Target layout of a service package
src/aws_analyzer/<service>/
__init__.py # module docstring (the old file's), public re-exports, __all__. Nothing else.
helpers.py # section 1: parsing and formatting
models.py # section 2: dataclasses
analysis.py # section 3: pure functions and *_findings -> analysis/ package once over the limit
analyzer.py # section 4: <Service>Analyzer, the only module that makes AWS clients
style.py # _ROOT, _EXTRA_CSS, _CSS, _BADGE, _render_html: the service's look
blocks.py # the service's own render blocks and their @_draws drawers -> blocks/ when big
view.py # <Service>View, its cell/row helpers -> view/ with _<Topic>Commands mixins when big
explorer/ # only if the service has a widget window: the window class, tabs as mixins, its CSS
Service-specific core modules go next to these, at the layer they belong to. Examples: readers/ (S3's file parsers),
http.py (OpenSearch's signed REST client), machine.py / envs.py (SageMaker).
Layers, lowest first. A module imports only from its own layer or lower ones:
| Layer | Modules | May also import |
|---|---|---|
| core | helpers → models → analysis → analyzer (and the service-specific core modules) | stdlib, boto3/botocore, core-safe _kit (+ render._call), _bedrock for the Bedrock pair |
| UI | style → blocks → view → explorer | anything in core, every _kit module |
analysisnever importsanalyzer. Pure functions make no AWS calls.viewimportsexploreronly lazily, insideexplore(), to avoid an import cycle.- Optional packages still load lazily (
_require), in every module. - Every module starts with a docstring and
from __future__ import annotations. The# ====section banners go away, since the module is the section now. - Mixin naming, so
rules.pycan find them:- View command mixins:
_<Topic>Commands. - Explorer tab mixins:
_<Topic>Tab, and_<Topic>Panefor a window without tabs (S3's list and report panes). - Analyzer mixins:
_<Topic>Reads. - The composed class lives in the module named for its layer (
view.pyorview/__init__.py, ...).
- View command mixins:
Phase 0: tooling (done)
The guardrails used to assume one file per analyzer. Now they check packages, and every later step relies on them.
This is what they do, so you know what a failing check means. All of it was tried on a throwaway copy of the repo
where dynamodb, s3 and _bedrock were packages, with a deliberate violation for each rule.
.claude/skills/check/rules.py:
- The read-only check covers the whole package. It finds the services an analyzer makes clients for across
every module of its package (
owner_services), plus its COMPANIONS parents', each a file or a package. So a module that callsclient.put_itemon a client another module made is still caught. - Parents and SHARED may be packages (
module_files,top_modules):s3/as the agent's parent,_bedrock/as the Bedrock pair's shared package. - Commands in mixins are checked. In a package, every class named
*View,*Explorer,_*Commandsor_*Tab(_ui_classes) gets the command checks (check_commands): a guard (@_friendly_errorson a View or_*Commands,@_window_errorson an explorer or_*Tab), a docstring,-> None, and noprint/display/HTML. - The layer rule (
check_layers). A core module that imports its package'sstyle/blocks/view/explorer/window, or a UI module of_kit(exceptrender._call/_signature), is an error, and so isanalysis*importinganalyzer*. The package's__init__.pyis exempt, since it re-exports both layers. - No copies inside a package (
check_package). A function or class defined in two modules of one package is an error. Constants may share a name. - No reaching into private names (
check_reach). A companion package (agent/) touching a private name of an analyzer or of its module (core._window,lf._x,s3._exposure) is a warning. It found exactly the nine in decision 7. It becomes an error in "the leftovers".
tests/test_style.py:
MAX_MODULE_LINES = 1500applies to every module undersrc/aws_analyzer/except those inLONG_FILES.LONG_FILESis the set of files that were longer than that before this plan. A file in it may still grow, since features keep landing in services that haven't been split yet.- The second test fails once a listed file is gone or fits the limit, so its entry gets removed.
.claude/skills/check/snapshot.py:
What
names.jsonrecords for a package:definedis the names in its__all__, which is its promise to users.insideis every name its modules define.How
comparejudges a missing name:- a private name that left the top level but is in
insidecounts asmoved, notLOST; - a public name missing from the top level is always
LOST.
No flag is needed.
- a private name that left the top level but is in
tests/test_package.py: a SHARED module may be a package.
Found on the way and fixed:
agent/toolkits/s3_tools.pyhad its own copy of_lambda_module. It now imports the one inlambda_tools.py.
The recipe for one service
One PR per service, branch modularize/<service>, one commit per bullet so each can be checked on its own.
.venv/bin/python if it exists, else python; $S below is the service.
Baseline, from main, before changing anything. Save both outputs in the scratchpad, not the repo:
python .claude/skills/check/snapshot.py run "$SCRATCH/base" --service $S python .claude/skills/check/rules.py --apis > "$SCRATCH/base-apis.txt"Make the private names the agent uses public (decision 7), if this service owns any. Rename the name and update every caller: the module,
agent/, tests,demo.pyandsnapshot.py's cases. Nothing else changes.File becomes a package:
git mv src/aws_analyzer/$S.py src/aws_analyzer/$S/__init__.py(git keeps the history). Then:- Fix relative imports:
from ._kit→from .._kit, andfrom ._bedrock→from .._bedrock. - Re-key the per-file config:
ruff.tomlper-file-ignores andCOMMENTS_LEFT. Swap the path inLONG_FILESforsrc/aws_analyzer/$S/__init__.py, since it's still the long file until step 4 empties it.
The full check must pass. This commit changes no line of code beyond those imports.
- Fix relative imports:
Split it with
split.py, in one commit. Never retype code: retyping thousands of lines lets in mistakes that tests can miss.Write the plan in the scratchpad,
$SCRATCH/$S_plan.py, the wayplans/opensearch.pydoes:MODULES: every top-level statement by name, in layer order;DOCS: each module's docstring;REEXPORT_KIT: the public_kitnames the old module imported, which__init__keeps exporting.
To list the statements, run
split.pywith an empty plan: it names the ones not placed. A name assigned twice at the top level is named with its line,_SECRET_NAME_RE@1669. In one file the last assignment wins for every function, so check what giving each half its own module changes before you place them. A bare expression statement (a drawer registered with_draws(...)(...)) is named by its line,expr@4291.A class over the limit becomes mixins first (decision 5), with
mixins.py. AddMIXINSto the same plan: the class, then each mixin's name, docstring and members, in the order they go in its bases. Keep__init__and anything that callssuper()on the class. An alias in the class body (_preview_tsv = _preview_csv) is a member named by its first name: put it in the mixin of the method it names, or the class can't be built. Then split its output instead of the file:python .claude/skills/modularize/mixins.py "$SCRATCH/$S.py" "$SCRATCH/$S_plan.py" "$SCRATCH/$S-mixed.py"The mixins are top-level classes now, so the plan places them like any other statement. Compare the sorted non-blank lines of the two files: only the mixins' headers and docstrings, and the class's new bases, should be new.
A lower layer that has to reach a higher one at run time imports it inside the function, and that's the one line a split may add. It happens when the View opens the window (
from .explorer.window import LambdaExplorerinLambdaView.explore()). Make the edit in the scratch copy before splitting, and say so in the commit.split.pydoesn't ask for a top-level import of a name a function imports itself.Split the file as it was before step 3 (
git show HEAD~1:src/aws_analyzer/$S.py), so its imports have their original depth, with the edits from 2 and 3 applied:python .claude/skills/modularize/split.py "$SCRATCH/$S.py" "$SCRATCH/$S_plan.py" "$SCRATCH/$S-out"It moves each statement verbatim with the comments above it, drops the
# ====banners, writes each module's imports from the names it reads (relative ones for its own package), and stops if a module would read a name from a later one at run time. Fix that by reordering the plan, or by moving the statement next to what it reads: a dataclass whose property calls an analysis function belongs in that analysis module, not inmodels.py. Otherwisemodelsandanalysiswould import each other. To see every such dependency at once, not one per run, loop over the plan withsplit.py'sread_source,defined_namesandused_names.Put the output in place of
src/aws_analyzer/$S/. Then remove the imports it wrote generously:ruff check src/aws_analyzer/$S --select F401 --fix.Print the config entries each module needs, and replace the one entry in
ruff.tomland inCOMMENTS_LEFTwith them:python .claude/skills/modularize/split.py --report src/aws_analyzer/$S C901,PLR0912,PLR0913,PLR0915Use the old entry's rules. Then take
$SoffLONG_FILES.Check:
ruff check .,python .claude/skills/check/rules.pyandpython -m pytest tests/test_$S.py tests/test_package.py tests/test_style.py -k "not docstrings" -q.
Fix the tests and scripts that reached into the old file:
monkeypatch.setattr(lfmod, "_RUN_LINES", 6)must now patch the module that reads the name (from aws_analyzer.lambda_functions.explorer import logs as logs_tab). Patching the package's re-export changes nothing, and the test then passes or fails for the wrong reason.- Private names a test imports come from their new module. Import the submodules under names that can't clash
with a test's local variables (
stylemod,wiring_block,logs_tab): ruff's F823 catches a local that shadows one. - A test that swaps a name by assignment (
lfmod.map_package = held_back) swaps it on the module that looks it up, the same as a monkeypatch. - Do the same in
.claude/skills/demo/*.pyand insnapshot.py'sDEMO_CASES.
Test files keep their names (
tests/test_$S.py). Splitting them is a separate change.Prove nothing changed:
python .claude/skills/check/snapshot.py run "$SCRATCH/new" --service $S python .claude/skills/check/snapshot.py compare "$SCRATCH/base" "$SCRATCH/new" python .claude/skills/check/rules.py --apis | diff "$SCRATCH/base-apis.txt" - # only paths may differEvery output must be identical, and no name
LOST. Private names that moved into a module are reported asmoved, which is fine. The snapshot'sapis.txtdiffers only by the file it names (<pkg>/$S.py→<pkg>/$S/analyzer.py): check that by diffing it with the path swapped back. Any other difference is a bug in the move: find it, don't accept it.Docs for agents:
- Update CLAUDE.md where it names this service's file (
lambda_functions.py's_EXPLORER_CSS, ...) so a path search lands in the right module. - Update the progress table below.
- No CHANGELOG entry: users see no change.
- Update CLAUDE.md where it names this service's file (
Run
/checkin full, then open the PR. The PR description says it's a move-only refactor, links this plan, and tells the reviewer to usegit diff main --color-moved=dimmed-zebra -M.
Config files that are keyed by file
These lists normally only shrink, and a new file is never added. A split is the one sanctioned exception, because the same code just changes file:
ruff.tomlper-file-ignores: replace the service's one entry with one entry per new module, listing only the rules that module actually breaks. Runruff check <module> --select <the old entry's rules>without the ignore to see which. Never a rule the old entry didn't have.COMMENTS_LEFT: one entry per new module, with its real count. The new counts must add up to no more than the old number. The# ====/# 1. ...banners aren't counted, so dropping them changes nothing.LONG_FILES: the old file's entry goes once the split is done, and no new module may be added to it.
Per-service plans
Line counts are from 0.20.0. A class over 1,500 lines needs mixins. The exact grouping is the agent's call, made by
reading the code, but follow the topics below and the View's _GROUPS.
opensearch (4,198 lines) — the pilot, done
It's small and has most of the layers, so it proves Phase 0's tooling and the recipe cheaply, and its PR stays easy
to review. It doesn't exercise mixins, an explorer window, the service's own blocks or the agent's private names.
lambda_functions comes next and proves those, so treat it as the second pilot. After each of the two, write what
went differently from this plan under "Lessons" at the end and fix the recipe, before the next service starts.
opensearch/
__init__.py
helpers.py # section 1 (~400 lines): parse_location, endpoint_service, parse_engine_version,
# version_at_least, vector_norm, vector_summary, values_at, _int, _bytes, ...
models.py # section 2 (~390): Domain, Collection, Endpoint, VectorField, IndexInfo, ClusterHealth,
# KnnStats, StoreReport, VectorCheck, Sample, Embedding, Hit, SearchResult, Overview
analysis/ # section 3 (~1,280): under the limit, but split by topic to prove the analysis/ pattern
parsing.py # the names tables, similarity_name, score_meaning, parse_domain, parse_collection,
# network_access, parse_mapping, unindexed_fields, read_settings, guess_text_field,
# source_field, source_of, index_from_cat, parse_knn_stats
vectors.py # vector_memory, vector_count, index_vector_memory, knn_memory_limit, domain_knn_memory,
# score_to_similarity, check_vectors
cost.py # domain_monthly_cost, serverless_minimum, ocu_monthly_cost
queries.py # the filter operators, build_filter, describe_filter, knn_query, query_python,
# describe_vector_field
findings.py # domain_findings ... search_findings (~450)
http.py # _READ_POSTS, OpenSearchError, _error_details, _path, _store_url
embeddings.py # _as_vector, _base_model, _embedding_style, _embedding_request, _embedding_from
analyzer.py # OpenSearchAnalyzer (~850)
style.py # _ROOT, _CSS, _BADGE, _render_html
view.py # the cell and label helpers (_state_tone ... _filter_rows) and OpenSearchView (~1,050)
request()and the signing stay methods ofOpenSearchAnalyzer. Taking them out of the class would change code, not move it.http.pyholds only the module-level REST helpers, andanalyzer.pyimports_READ_POSTSfrom it. CLAUDE.md's description ofrequest()stays true._existsand_drop(in section 4, before the class) go to whichever module their callers are in.- A constant that both the view and the analysis read (the names tables,
_FILTER_OPS, ...) goes in the lower layer, and the view imports it from there. - No first commit for decision 7: the agent has no OpenSearch tools.
- Tests:
osmod._bytesandosmod._intcome fromaws_analyzer.opensearch.helpers, andosmod._render_htmlfromaws_analyzer.opensearch.style. There are no monkeypatches to re-target.tests/fake_opensearch.pyand demo.py'sseed_opensearch()use public names only, but grep them anyway.
lambda_functions (11,890 lines) — second, the full pilot, done
The layout below was the plan. What it became is plans/lambda_functions.py (34 modules, the largest view.py at
1,450 lines). It differs in these ways:
prices.pyholds the price table.analysis/text.pyholds the findings' wording helpers (_window,_cli).analysis/overview.pyholdsOverview.LogRun/LogPagelive inanalysis/logs.py, the code map's dataclasses inanalysis/codemap/model.py, and the graph's inanalysis/codemap/graph.py: each next to the functions its properties call.analysis/codemap/hasterms.py(what the reader knows) andsyntax.py.- The explorer is
frame.py,widgets.py, a module per tab andwindow.py.
It has every kind of front-end (View, explorer, agent tools), so it proves what OpenSearch couldn't: mixins, an explorer, blocks, and decision 7.
lambda_functions/
helpers.py # section 1 (lines ~100-320)
models.py # section 2: Function, Invocation, Overview, LogEvent, CodeMap, CodeStep, CodeGraph, ...
analysis/
functions.py # masked_config, parse_function, runtime_status, parse_policy, parse_event_source_mapping
cost.py # provisioned_monthly_cost, log_storage_monthly_cost, function_monthly_cost
findings.py # function_findings, account_findings, _runtime_findings, _usage_findings, _errors_level
errors.py # request_id_of, classify_error, group_errors, error_findings
logs.py # parse_report, split_runs, line_level
performance.py # percentile, suggest_memory, performance_findings
package.py # handler_file, package_file_name, read_package, package_findings
codemap/ # ~2,300 lines on its own
aws_model.py # _service_name, _botocore_loader, _aws_model, _iam_action, events, secrets
reader.py # _Sym, _Frame, _Module, _Collect, _CodeReader (~1,060 lines)
mapping.py # map_code, map_package
findings.py # code_map_findings, _code_summary
graph.py # code_graph, the mermaid helpers
analyzer.py # LambdaAnalyzer (~1,130 lines)
style.py
blocks/
wiring.py # _Wiring, _Columns and their drawers
outline.py # _Outline, _flow_html, _outline_html/_lines
graph.py # _Graph, _GCard, the _g_* SVG layout and drawing (~700 lines)
view.py # LambdaView (~1,180 lines) and its cell helpers (_label, _runtime_meaning, ...)
explorer/
__init__.py # LambdaExplorer composed from its tabs, explore()
window.py # _explorer_rules, _EXPLORER_CSS, _window_text, _FunctionField, _Row, _RunRow
functions.py logs.py code.py ... # LambdaExplorer (~2,150 lines) as _<Topic>Tab mixins
- First commit (decision 7):
LambdaAnalyzer._window→windowand._numbers→numbers. Pick clearer names if they collide with an attribute; every caller changes. - Tests to re-target:
_RUN_LINES,_TOGGLE_GRACE,map_package(patched where the explorer looks it up),urllib.request.urlopen(on the module that calls it).
s3 (11,679) and s3_explorer (2,932), done
The layout below was the plan. What it became is plans/s3.py (51 modules, the largest view/folders.py at 860
lines) and plans/s3_explorer.py (s3/explorer/, 8 modules). It differs in these ways:
readers/comes aftermodels(the parsers returnDocument,Picture,PdfPage) and holds formats, avro, office (Word, PowerPoint, Excel), pdf, media and data;prices.pyholds the price tables andARCHIVE_CLASSES.analysis/is cost, summary, bucket (bucket_findings and the settings in words the agent reads), duplicates, downloads, lifecycle, policy, notifications, documents (pdf_flow) and details (file_details()'s readers).S3Analyzerkeeps its plumbing (__init__, regions, clients,account_id) inanalyzer/s3_analyzer.py, over_BucketReads,_ListingReads,_VersionReads,_FileReads,_PreviewReadsand_DownloadReads, a module each;threads.pyandstreams.pyhold the module-level helpers.S3View(view/s3_view.py) is a_<Topic>Commandsmixin per_GROUPSentry, inview/buckets.py...view/downloads.py, each with the cell helpers only it uses;view/cells.pyhas the two that several share.S3Explorer(explorer/window.py) is_ListPane,_ReportPane,_SelectionPaneand_SettingsPane, withnavigator.py(the pure helpers andS3Navigator) andframe.py(stylesheet, rows).helpers.pyplusreaders/: the stdlib-only Avro, DOCX and PPTX parsers and format sniffing (section 1 is 1,440 lines, mostly these).analysis/: summary, lifecycle (simulate_lifecycle), policy (explain_policy), cost, findings.S3Analyzer(~3,590 lines) →analyzer/with_<Topic>Readsmixins: listing, configuration, files and preview, downloads (clean_downloadsand the downloads-folder guards stay together in one module, because they are the one place that deletes files), CloudWatch.S3View(~2,460 lines) →view/with_<Topic>Commandsmixins by_GROUPS.Fold
s3_explorer.pyin ass3/explorer/(S3Navigator and the pure helpers inexplorer/navigator.py).aws_analyzer/s3_explorer.pystays as a module that re-exportsS3ExplorerandS3Navigator, so old imports work. Along with it:- Remove
s3_explorerfromCOMPANIONS. - Point
_EXPORTSforS3ExplorerandS3Navigatorats3. - Update
test_explorer_builds_on_the_s3_installed_with_itand the bare-name list intests/conftest.py. _s3_module()becomes a plain import of the parent package. Itsimport s3/__main__fallbacks date from the standalone files and go.
This is the one step in the plan that changes a test's expectation (where
S3Exploreris defined). Say so in the PR.- Remove
First commit (decision 7):
S3Analyzer._cloudwatch_in, ands3._rule_scope,_rule_actions,_exposure,_encryption_label,_block_label.
_bedrock.py (2,078) → _bedrock/, done
The layout below was the plan. What it became is plans/_bedrock.py (11 modules, the largest markdown.py at 730
lines). It differs in these ways:
- The dataclasses (
KBDocument,Passage,Citation,ModelInfo) are inmodels.py, andparse_modelswith its helpers infoundation_models.py, soprices.pyholds only the tables,model_price,short_modeland the rerank models. helpers.py(IDs, source names, Bedrock's metadata keys, name matching) andwords.py(the question's words, snippets and_highlight) are new.markdown.pyis pure (it imports only_kit.text), andblocks.pyis the one UI module:_Answer,_Json,_Download, their drawers and_file_list_blocks.
prices.py (price tables, DEFAULT_MODEL, model_price, short_model, parse_models, ModelInfo), parsing.py
(Passage, Citation, KBDocument, parse_retrieve, parse_document), filters.py, estimates.py
(cost and tokens), markdown.py (_Markdown, _Answer, _Json and their drawers: UI layer), files.py
(presigned_url, the file lists, _Download). SHARED and rules.py's shared-name check follow the package.
bedrock_kb (8,566), done
The layout below was the plan. What it became is plans/bedrock_kb.py (34 modules, the largest explorer/window.py
at 1,060 lines). It differs in these ways:
BedrockKBAnalyzerwas 1,370 lines, under the limit, but took mixins by its own sub-banners:_KnowledgeBaseReads(knowledge bases and data sources),_SearchReads(retrieval and generation),_DecidingReadsand_FileReads, inanalyzer/, withkb_analyzer.pykeeping__init__andclient.BedrockKBViewis_KnowledgeBaseCommands,_IndexCommands("What's indexed", 840 lines),_SearchCommandsand_MeasureCommandsby_GROUPS, inview/;kb_view.pykeeps the plumbing andexplore(), andview/cells.pythe labels and tones the View and the window share.analysis/is common, prompting, parsing, settings, filters, deciding, cost, findings, files, metadata and chunks.- The blocks fit in one
blocks.py(290 lines), and the explorer isframe.py,widgets.pyandwindow.py.
analysis/: inventory and file states (inventory_files, file_state, FILE_STATES), chunks (place_chunks,
order_chunks, chunk_overlap), retrieval and answers, findings. BedrockKBAnalyzer (1,950) → 2,585) → _<Topic>Reads
mixins (configuration, files, search). BedrockKBView (_<Topic>Commands. Blocks (_Steps,
_Pipeline, _Chunks, _Shares, _Passage) into blocks/. KBExplorer (~1,000) → explorer/.
bedrock_chat (9,079), done
The layout below was the plan. What it became is plans/bedrock_chat.py (34 modules, the largest analyzer.py at
920 lines). It differs in these ways:
BedrockChatAnalyzer(880 lines) stayed one module, with no mixins.analysis/is parsing, schema, filters, choices (what the pickers list), request, findings, batches, sweeps, ranking, records and code.Fieldlives inanalysis/schema.py,Batch/BatchIteminanalysis/batches.pyandSweepinanalysis/ranking.py, each next to what its properties call;_diffwent tohelpers.py.BedrockChatViewis_ChatCommands,_TestCommands,_RunsCommands,_SettingsCommandsand_KnowledgeBaseCommandsby_GROUPS, inview/;chat_view.pykeeps the plumbing andchat()._ChatAppis_PickersPane,_ConversationPane,_SettingsTab,_RequestTab(Request, Response and Code),_TestTaband_RunsTab, inwindow/;app.pykeeps__init__, the layout, the plumbing and the public methods the View calls (sync,added,cleared,ran,swept,loaded,runs_changed).blocks.pyalso holds what the View and the window both draw with (_wrap,_kind_text,_setting_marks,_QUICK), since the View can't import the window.
analysis/: the request schema (request_schema, Field, Schema, _KNOWN, build_request,
settings_from_request, validate_request), runs and sweeps (parse_questions, sweep_setups, rank_runs,
run_score, ranking_findings), the code it writes (python_script, config_json, cli_command), and saved runs
(run_record, read_runs). _ChatApp (2,020) → 2,230) →
window/ with a _<Topic>Tab mixin per side tab (settings,
test, runs, code, request/response) and one for the pickers and composer. BedrockChatView (_<Topic>Commands.
sagemaker_env (5,044), done
The layout below was the plan. What it became is plans/sagemaker_env.py (14 modules, the largest view.py at
1,140 lines). It differs in these ways:
prices.pyholdsINSTANCE_TYPESandSAGEMAKER_PRICES.machine.pyandenvs.pyareSageMakerAnalyzer's mixins (_MachineReads,_EnvReads), with the constants only they read (METADATA_FILE,_HOMES,_PACKAGES).create_envand_runareenvs.py's_EnvMaking, a mixin of their own.analyzer.pykeeps__init__, the clients, the notebooks andrunning().- The pure side of each is in
analysis/:machine(the/proc, nvidia-smi and metadata parsers, which return section-2 dataclasses, so they left section 1),common(what several reports' findings share),notebooks,disk,runningandenvs. shell_pathand_tildemoved from section 3 tohelpers.py, since the disk, environment and View code all write paths with them.
The core has three parts, not one:
machine.py:/proc, disks, GPU, the metadata file,_shown/_real.envs.py: environments, kernels, tools,env_plan.analyzer.py: AWS.
create_env and _run are the module's only writes, so keep them together in one module with nothing else that
writes. The View (~1,055) stays one module.
dynamodb (3,499), done
The layout below was the plan. What it became is plans/dynamodb.py (7 modules, the largest view.py at 1,300
lines). It differs in one way: prices.py holds DYNAMODB_PRICES, as in every other package.
A flat package, like the template, with no mixins expected. DynamoDB JSON ↔ Python conversion stays in
helpers.py.
Last: the leftovers
- Update the
new-analyzerskill to scaffold a package from the template, with dynamodb as its model. - Update CLAUDE.md: remove "Architecture of an analyzer file" and the five-sections rule in
rules.py. - Update README / docs wherever they say "each service is one module", and the module name each guide's eyebrow
and
docs/index.mdshow (<code>opensearch.py</code>). These were left alone so the split wouldn't collide with the docs rewrite ondocs-skimmable. - Make the private-names warning from Phase 0 an error.
Phase 2 (needs its own plan and the user's go-ahead): findings that carry actions
Today a finding is ("warn", "... (see errors('etl'))"). The View call is written into the sentence, and each
window rewrites it with regexes (_window_text, _WINDOW_WORDS) into "the Errors tab". The core is speaking one
front-end's language. The direction is a core-side Finding(level, message, action) with
Action(command, args, kwargs), rendered by each front-end: the View through _call, a window as its tab, the
agent as a _next.
It isn't part of the moves because:
- It changes what
*_findingsreturn, which is public API. - CLAUDE.md says the pairs stay as they are.
Write that plan only after the moves are done, and ask the user before starting it.
Progress
Update this table in each step's PR.
| Step | Status | PR | Notes |
|---|---|---|---|
| Phase 0: tooling | done | #97 | branch modularize/tooling |
| opensearch | done | #98 | plan in plans/opensearch.py |
| lambda_functions | done | #99 | plan in plans/lambda_functions.py (mixins and modules) |
| s3 + s3_explorer | done | #101 | plans in plans/s3.py and plans/s3_explorer.py; branch modularize/s3 |
| _bedrock | done | #107 | plan in plans/_bedrock.py; branch modularize/_bedrock |
| bedrock_kb | done | #108 | plan in plans/bedrock_kb.py (mixins and modules); branch modularize/bedrock_kb |
| bedrock_chat | in review | #114 | plan in plans/bedrock_chat.py (mixins and modules); every module now fits the limit, so LONG_FILES is empty |
| sagemaker_env | done | #105 | plan in plans/sagemaker_env.py (mixins and modules); branch modularize/sagemaker_env |
| dynamodb | in review | #109 | plan in plans/dynamodb.py; branch modularize/dynamodb |
| leftovers | not started | new-analyzer skill, CLAUDE.md, README, private-names error | |
| Phase 2: findings with actions | needs a plan | ask the user first |
Mistakes to avoid
- Mixing a fix into a move. If you spot a bug while moving code, note it for a follow-up PR. A move commit that also changes code is hard to review, and the snapshot can't tell the two apart.
- Patching the re-export.
monkeypatch.setattr(package, name, ...)doesn't reach a submodule that imported the name. Patch the module that looks it up. - Import cycles. If
view.pyandexplorer/import each other at the top, the package fails to import. Lower layers never import higher ones, andexplore()imports the explorer inside the function. - Leaving
__init__.pyheavy. It holds the docstring, the imports for re-export and__all__. No logic, no constants, no helper "for now". - Losing an AWS operation from
rules.py --apis. That means the read-only check no longer sees a call. Fix the services lookup; don't accept the diff. - Raising a number in
COMMENTS_LEFT,LONG_FILESor a per-file-ignores entry to make a check pass.
Lessons
What a finished step found that this plan got wrong or didn't say, newest first, with the service it came from. Fix the recipe above as well. This section is for what the next agent must know and can't see in the code.
bedrock_chat:
- A window's public methods that the View calls aren't commands.
_ChatApp.sync(),ran(), ... have no@_window_errors, andrules.pychecks every public method of a_*Tab/_*Paneas a command. They stayed in the composed class (_ChatAppisn't named*Explorer, so it isn't checked), and the mixins hold only private methods. Adding the guard would change behaviour, so it's not part of a move. - A top-level bare expression (
_draws(_Code, "text")(_text_lines), a drawer registered without adef) had no name a plan could use.split.pynow names oneexpr@<line>. - A test patched a name two modules look up (
_cell_number:app()records it,_ipython_display_compares with it). Patching one would make the test pass for the wrong reason;in_cell()patches both. - mixins.py writes the new class line as one line, which went past 120 characters for
BedrockChatView. Wrap it in the scratch copy before splitting, as with the lazy import. - Nothing for decision 7: the agent doesn't read bedrock_chat.
- A window's public methods that the View calls aren't commands.
dynamodb:
rules.py --apislistsdynamodb:Scan/Queryundermodels.pyandview.py. Strings that name an operation (operation: str = "scan",_GROUPS's"query",_call("scan", ...)) count as one unless they're a keyword value or compared. In one file they merged with the analyzer's real calls; split apart they show. The union per service is the baseline's, which is what to compare.- Nothing for decision 7: the agent has no DynamoDB tools.
bedrock_kb:
REEXPORT_KITtakes any public name the old module imported, not only_kit's:split.pyfinds where each came from in the old imports. bedrock_kb's_bedrocknames (MODEL_PRICES,Passage, theX as Xones) are listed there, sobedrock_kb.MODEL_PRICESstill works.split.pyrewrites the imports, so a comment above them is dropped. bedrock_kb's two lines above its_bedrockimports went, andCOMMENTS_LEFT's total went down by two.- Module-level helpers in section 4 that several mixins call (
_question_text,_eval_pairs,_with_errors) were pure, so they went toanalysis/, not into one mixin's module. - A test patched a stdlib module through the service (
monkeypatch.setattr(kbmod.time, "monotonic", ...)): it's the same module object, so the test importstimeitself. apis.txtlists the operations per module now: compare the union per service with the baseline's (a short script over both files), as s3's lesson says.
_bedrock:
- Its users imported its private names (
from ._bedrock import _highlight, _Answer, ...), and a package's__init__re-exports only public names. bedrock_kb, bedrock_chat and the agent now import each name from the module that defines it (from ._bedrock.words import _highlight), theirX as Xre-exports kept; that was the one change outside the package. When bedrock_kb or bedrock_chat is split,split.pycarries those imports over with the right depth (from .._bedrock.words import ...). - No snapshot covers it on its own, and
names.jsonskips modules whose name starts with_. Run the snapshot for bedrock_kb and bedrock_chat (whose names include what they import from it), the agent's tests, and comparedir(aws_analyzer._bedrock)before and after against the names its submodules define. rules.pyread the package's__all__as a shared name and flagged every other__all__as a copy of it.shared_names()now leaves dunders out and says which module has each name.
- Its users imported its private names (
sagemaker_env:
- An analyzer under the limit can still take mixins, when the plan splits it by where it reads from. They
followed the class's own
# ----sub-banners (this machine, disk, Python environments), which moved with the first member under each, so the comment count didn't change. - A test that patched a module through the service (
smmod.platform) patches the same module object through the submodule that calls it (envs_reads.platform);_kitnames a test read off the service (smmod._Hint,smmod._Title) come from_kit. - Patching the class still works for mixins. demo.py sets
SageMakerAnalyzer._disk_usageand friends on the composed class, which shadows the mixin's method, so it needed no change.
- An analyzer under the limit can still take mixins, when the plan splits it by where it reads from. They
followed the class's own
s3:
- The explorer couldn't stay a companion through the split. It reached into s3 through
self.s3._Note,self.s3._render_html,self.s3._zip_layout, ..., and a package re-exports public names only. Its own commit (before the split) made it part of the package: those names are imported from_kitand the parent package instead,_s3_module()is the parent package, andx.s3stays. Do the same for any window that reads its service through a module object. - An alias module (
s3_explorer.py) keeps the old import path.rules.pychecks it holds nothing but re-exports (ALIASES), andtest_packagelets it load its target. The snapshot's compare lists the old file's private names as LOST, since a file module has no "inside"; check each one is in the new package'sinsidein names.json (they all were) instead of--allow-lost-ing them. - Parsers in section 1 returned section-2 dataclasses, and pure analysis read section-4 names (
_READ_ERRORS,_ReadLimit,_columnar_info,_npy_header).readers/went betweenmodelsandanalysis, and those names moved down into it.split.pystops on each such case; the deps listing (read_source,defined_names,used_namesover every statement and every member of the big classes) showed them all at once. - Class-body aliases (
_preview_tsv = _preview_psv = _preview_csv) stayed behind in the class and broke it.mixins.pynow treats one as a member named by its first name. - A rename for decision 7 can collide with a local: the View has
exposure = _exposure(...), so_exposurebecamepublic_exposure. The View helpers the agent reads moved to the core (analysis/bucket.py), since the agent reads only the core. - The tools assumed one file:
snapshot.py's_layoutlooked fors3.py, andshots.pyfound the View by__module__ ==(so it couldn't find a package's). Both fixed.rules.py --apisnow lists S3's operations per module; compare the union per service against the baseline, not the lines. - CI's wheel smoke test (
ci.yml, and the same inrelease.yml) nameds3_explorer._s3_module, which local/checkdidn't catch, sincerun.pyhas its own copy of the check. Grep.github/workflows/for the names a step moves, too. --allow-losttakes several names after one flag (--allow-lost a b c); repeating the flag keeps the last.
- The explorer couldn't stay a companion through the split. It reached into s3 through
lambda_functions:
- Some dataclasses call analysis functions in their properties (
Overviewreadsfunction_monthly_cost,LogPagereadssplit_runs). In one file that works because names are looked up when they're called. Split apart,modelsandanalysiswould import each other. They moved next to the functions they call, which keeps the layers acyclic without changing a line. Expect the same in every big service. - A name was assigned twice at the top level.
_SECRET_NAME_REwas the environment-variable pattern forsecret_like()and, 1,200 lines later, the code reader's. The second won everywhere. The two patterns match the same names, so giving each its own module changed nothing, but check this every time. Follow-up: one pattern, one name. - Mixins by tab worked.
LambdaExplorer's methods were already grouped by tab, so each mixin is a run of them, and a constant only one tab reads moved into that tab's module. A test then patches it there (logs_tab._RUN_LINES). Rules now check the explorer's public commands, and they already had@_window_errors. - Reaching up a layer took one lazy import, in
LambdaView.explore(). It's the only line in the split that isn't a move. - The OpenSearch PR left a dead link in README's services table (
opensearch.py). Grep README for the old file name, not only the line the recipe names. rules.py --apisshowed a falselambda:ListFunctionsinview.py. The view callsself.core.list_functions(), the analyzer's own method, and the check skipped method names only when the same file defined them. It now reads every function the package defines (owner_functions). If a split's API list gains a line in a UI module, look for the same thing before anything else.view.pyis 1,450 lines, close to the limit. The next command that grows it should split it into_<Topic>Commandsmixins by_GROUPS.
- Some dataclasses call analysis functions in their properties (
opensearch:
One commit for the whole split, not one per module. Code moved by
split.pychanges in no other way, and the snapshot checks every module at once. The recipe (step 4) now says so.The old module's docstring stays as written in
__init__.py, even where it says "this file". It's whathelp(aws_analyzer.opensearch)shows, and rewording it is a change of its own.__init__re-exports the public_kitnames the old module imported (human_size,HOURS_PER_MONTH, ...), sinceaws_analyzer.opensearch.human_sizeworked before. Stdlib and botocore names it imported (datetime,ClientError) aren't re-exported. Nothing used them, and they were never its API.Two modules were added to the layout:
prices.py(core), for the price tables. They were 240 lines of section 1, and a price check touches only them.embeddings.py(core), for building Bedrock embedding requests.
The analyzer-only helpers
_exists/_dropstayed inanalyzer.py.A view importing a core module's private helper (
_key_label,_is_serverless,_base_model) is fine: they're in the same package.
Phase 0:
test_docstrings_have_their_quotes_on_their_own_linestakes about 100 s, almost all of it on the long files (ast.get_source_segmentslows down on very long files). Expect it to speed up as services are split. Until then, runpytest tests/test_<service>.pywhile you work, and the full suite at the end.