refactor: incremental structural cleanup across expression, loader, and generator layers by cherninkiy · Pull Request #53 · protocollab-co/protocollab

cherninkiy · 2026-04-19T21:29:36Z

Summary

Incremental, behaviour-preserving refactoring of the largest production
modules in src/protocollab. Each step was followed by a targeted test
run; no public API or observable behaviour was changed.

Changes by scope

`expression/` (parser · evaluator · validator · ast_nodes)

8248e0e parser – extract _parse_left_associative(), replacing six
copy-pasted precedence methods (_bitwise_or … _mult).
9ce0804 evaluator – extract collection helpers (_evaluate_sequence,
_evaluate_dict_pairs, _iter_comprehension_contexts,
_evaluate_comprehension).
b5abba6 evaluator – extract runtime helpers (_evaluate_attribute,
_evaluate_subscript, _evaluate_binop, _evaluate_match).
4b3807b validator – extract traversal helpers (_collect_name_nodes,
_collect_name_pairs, _validate_nodes, _validate_pairs).
11cf2a7 validator – extract rule helpers (_collect_names_from_*,
_validate_*_node, _parse_expr_for_validation).
5626ea8 ast_nodes – add container-type aliases (ASTNodeSequence,
ASTNodeList, ASTNodePair, ASTNodePairs); apply to List, Dict,
ListLiteral, DictLiteral.
13b0826 ast_nodes – add scalar aliases (LiteralValue,
OptionalASTNode, ComprehensionKind, UnaryOperator, BinaryOperator);
apply across all relevant nodes.

`generators/`

6f46e69 lua_generator – split monolithic _compile_lua_expr()
dispatcher into _compile_lua_list, _compile_lua_dict_pairs,
_compile_lua_comprehension, _compile_lua_match.
4ee6a25 python_generator – extract _process_field(raw) for type
lookup, str special-casing, and GeneratorError raises.
e1bc261 python_generator – extract _render_output(context, output_dir, proto_id) to isolate Jinja2 env creation and file I/O.

`loader/`

2920f15 – extract factory helpers (_build_loader,
_should_use_isolated_loader, _build_isolated_loader,
_load_with_loader).
30235cb – add _select_loader(config, use_cache);
load_protocol() reduced to a single delegating call.

`main.py` (CLI)

3de00e9 – extract _ensure_input_file_exists, _build_load_config,
_print_validation_result, _register_generate_command; generate
sub-commands registered declaratively.

Test coverage

All targeted suites passed after every commit:
test_expression_*, test_python_lua_generators, test_loader,
test_cli, test_type_system.

- extract reusable parser helper for left-associative operators - clean up lexer/validator small inconsistencies - keep expression behavior unchanged

- extract list/dict/comprehension/match Lua compilation helpers - document sparse table behavior in Lua template - add regression coverage for filter comprehension generation

- extract shared helpers for file checks and validation output - build load config through a dedicated helper - register repetitive generate subcommands declaratively

- reuse sequence and dict construction helpers - centralize comprehension iteration and evaluation - keep evaluator semantics unchanged

- extract attribute, subscript, binop and match helpers - shrink the main evaluate dispatcher - preserve existing expression runtime behavior

- extract common helpers for AST node lists and key/value pairs - reduce duplicate recursion in name collection and validation - keep validator behavior unchanged

- extract dedicated helpers for comprehension and match validation - separate parse-to-errors orchestration from validate_expr - preserve existing validator behavior

- extract common aliases for repeated AST node collections - reuse shared tuple/list/pair types across literal nodes - keep expression AST API unchanged

- reuse aliases for comprehension kind and operator fields - reduce repeated string typing in AST dataclasses - keep expression AST behavior unchanged

- extract helper functions for loader construction decisions - centralize isolated-loader setup for load_protocol - preserve loader public API behavior

- choose global vs isolated loader through a shared helper - make load_protocol orchestration smaller and easier to follow - preserve public loader behavior

- Add _process_field(raw) that encapsulates type lookup, str special-case handling, and GeneratorError raises for missing/unsupported types - generate() loop now delegates to _process_field and skips empty results

- Add _render_output(context, output_dir, proto_id) that owns Jinja2 env creation, template loading, directory creation, and file write - generate() builds context dict and delegates I/O to _render_output

Copilot

Pull request overview

This PR performs a broad, behavior-preserving refactor across the protocollab expression subsystem (lexer/parser/evaluator/validator/AST), loader selection logic, and the Lua/Python generators, primarily by extracting helper functions and tightening internal structure.

Changes:

Refactors expression parsing/evaluation/validation by extracting reusable traversal and evaluation helpers, plus adds AST type aliases.
Refactors generators by splitting large Lua expression compilation dispatch and extracting Python field processing + Jinja2 rendering helpers.
Refactors loader and CLI entrypoints by extracting selection/build helpers and consolidating CLI command registration/printing logic.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/protocollab/type_system/primitives.py	Adds clarifying comment for `u3`/uint24 struct handling.
src/protocollab/tests/generators/test_python_lua_generators.py	Adds regression test for Lua `filter(...)` comprehension compilation.
src/protocollab/main.py	Extracts CLI helper functions for file checks, config building, validation printing, and generate subcommand registration.
src/protocollab/loader/init.py	Extracts loader construction/selection helpers; `load_protocol()` delegates to a single call.
src/protocollab/generators/templates/lua/dissector.lua.j2	Adds explanatory comment about sparse numeric-table membership semantics.
src/protocollab/generators/python_generator.py	Extracts `_process_field()` and `_render_output()` to simplify generator flow.
src/protocollab/generators/lua_generator.py	Splits `_compile_lua_expr()` into focused helpers; adjusts instance ordering logic.
src/protocollab/expression/validator.py	Extracts traversal and validation helpers; centralizes builtins set.
src/protocollab/expression/parser.py	Extracts left-associative parsing helper; adjusts grammar to parse `in` at correct precedence; strengthens `match` parsing diagnostic.
src/protocollab/expression/lexer.py	Exposes keyword set as `KEYWORDS`; modernizes typing.
src/protocollab/expression/evaluator.py	Extracts evaluation helpers for sequences/dicts/comprehensions/attribute/subscript/binop/match.
src/protocollab/expression/ast_nodes.py	Adds and applies AST type aliases for containers and scalar concepts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- main.py: move strict-warning handling into the is_valid+warnings branch so it is reachable; exit non-zero when --strict and warnings exist - evaluator.py: annotate _iter_comprehension_contexts return type as Iterator[tuple[Any, dict[str, Any]]] instead of Any

Replace named captures that were immediately discarded (F841) with bare class-pattern matches e.g. Comprehension() instead of Comprehension(kind=kind, expr=expr, ...) in evaluator and validator.

cherninkiy added 13 commits April 20, 2026 00:02

refactor(expr): reduce parser and validator duplication

8248e0e

- extract reusable parser helper for left-associative operators - clean up lexer/validator small inconsistencies - keep expression behavior unchanged

refactor(lua): split expression codegen helpers

6f46e69

- extract list/dict/comprehension/match Lua compilation helpers - document sparse table behavior in Lua template - add regression coverage for filter comprehension generation

refactor(cli): simplify command wiring

3de00e9

- extract shared helpers for file checks and validation output - build load config through a dedicated helper - register repetitive generate subcommands declaratively

refactor(expr): extract evaluator collection helpers

9ce0804

- reuse sequence and dict construction helpers - centralize comprehension iteration and evaluation - keep evaluator semantics unchanged

refactor(expr): isolate evaluator runtime helpers

b5abba6

- extract attribute, subscript, binop and match helpers - shrink the main evaluate dispatcher - preserve existing expression runtime behavior

refactor(expr): share validator traversal helpers

4b3807b

- extract common helpers for AST node lists and key/value pairs - reduce duplicate recursion in name collection and validation - keep validator behavior unchanged

refactor(expr): isolate validator rule helpers

11cf2a7

- extract dedicated helpers for comprehension and match validation - separate parse-to-errors orchestration from validate_expr - preserve existing validator behavior

refactor(expr): share AST container aliases

5626ea8

- extract common aliases for repeated AST node collections - reuse shared tuple/list/pair types across literal nodes - keep expression AST API unchanged

refactor(expr): share AST operator aliases

13b0826

- reuse aliases for comprehension kind and operator fields - reduce repeated string typing in AST dataclasses - keep expression AST behavior unchanged

refactor(loader): share loader factory helpers

2920f15

- extract helper functions for loader construction decisions - centralize isolated-loader setup for load_protocol - preserve loader public API behavior

refactor(loader): isolate loader selection

30235cb

- choose global vs isolated loader through a shared helper - make load_protocol orchestration smaller and easier to follow - preserve public loader behavior

refactor(python_gen): extract field processing helper

4ee6a25

- Add _process_field(raw) that encapsulates type lookup, str special-case handling, and GeneratorError raises for missing/unsupported types - generate() loop now delegates to _process_field and skips empty results

refactor(python_gen): extract template rendering helper

e1bc261

- Add _render_output(context, output_dir, proto_id) that owns Jinja2 env creation, template loading, directory creation, and file write - generate() builds context dict and delegates I/O to _render_output

cherninkiy requested a review from Copilot April 19, 2026 21:29

Copilot started reviewing on behalf of cherninkiy April 19, 2026 21:30 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Comment thread src/protocollab/main.py

Comment thread src/protocollab/expression/evaluator.py

cherninkiy added 3 commits April 20, 2026 00:37

style: apply black formatting to 4 files

5e52027

fix(flake8): remove unused variables in match-case branches

18186f3

Replace named captures that were immediately discarded (F841) with bare class-pattern matches e.g. Comprehension() instead of Comprehension(kind=kind, expr=expr, ...) in evaluator and validator.

cherninkiy merged commit 03b1c09 into main Apr 19, 2026
6 checks passed

cherninkiy deleted the dev branch April 19, 2026 21:57

Navigation Menu

Uh oh!

refactor: incremental structural cleanup across expression, loader, and generator layers - #53

refactor: incremental structural cleanup across expression, loader, and generator layers#53
cherninkiy merged 16 commits into
mainfrom
dev

cherninkiy commented Apr 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cherninkiy commented Apr 19, 2026

Summary

Changes by scope

expression/ (parser · evaluator · validator · ast_nodes)

generators/

loader/

main.py (CLI)

Test coverage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`expression/` (parser · evaluator · validator · ast_nodes)

`generators/`

`loader/`

`main.py` (CLI)