docs: trim source prose to contracts; drop closure-era rationale - #482
Merged
Conversation
Brings every module under `modern_di/` (except `exceptions.py`, out of scope) to the AGENTS.md rule: public API docstrings state the contract, internal helpers get a one-line contract, and rationale lives in one place rather than a copy at each site it applies to. Removed: the comments that cited an invariant test to justify an inlined lookup. The frame-budget rationale they repeated already lives in `test_resolve_costs_exactly_one_resolver_frame_per_node`, which is unchanged. Kept, as the two comments a reader would otherwise read as a bug: the scope map holds ancestors only (a self-entry is a reference cycle), and the `_next_deeper` memo is keyed by enum type as well as member (IntEnum members hash by value). Rationale that was recorded nowhere else, moved rather than deleted: - Container `__init__` "inlined, not a helper" keeps a one-line pointer, now at the benchmark that measures it (`test_g6_build_child_container`). The spec expected the invariant test it cited to be its home, but that test measures the resolve path; nothing measures child-container construction. - `_handle_recursion_error`'s reason for being a separate call (the coverage tracer re-arms on the fresh boundary) moved into its docstring. - `find_context`'s `in`/`[]` over `.get(key, UNSET)` keeps one line: `.get` skips a dict subclass's `__contains__`/`__getitem__`, and no test covers it. - "a root seeds `container_provider`" was promoted from a comment into the `__init__` docstring, where it is a contract: `resolve(Container)` works. Test prose: the three provider test modules whose commentary described the per-resolver "override front-guard" and the compiled closures now speak of the compiled constant resolver and the generated template. Two modules beyond the three the spec named are also touched, both for dangling references #480 created: the arity comment in `test_resolver_compiler.py` described a 0/1/ generic closure split the template replaced, and five citations of deleted `docs/adr/2026-*.md` files (four in `test_container.py`, one in `test_context_provider.py`) now name the design-decisions section that absorbed the decision, "## 6. Validation is explicit". ADR-0030's free-threading sentence needed no correction: #480 deleted that record, and `docs/introduction/design-decisions.md` already states the fact. No signature, public attribute, exception class, `__all__` or docs slug moves. Verified two ways: every module's AST with docstrings stripped is identical before and after, for `modern_di/` and for `tests/`; and a dump of the exported names, signatures, MROs and `docs_slug` values is identical against main. Prose share of `modern_di/`: 25.4% -> 18.6% of non-blank lines; excluding the out-of-scope `exceptions.py`, 24.7% -> 15.7%. The package figure stays above the 12-15% the spec targets: `exceptions.py` is 158 prose lines that this change may not touch, 6.6 points of the package total on its own. Closes #472
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: 08c2333 | Previous: c082427 | Ratio |
|---|---|---|---|
benchmarks/test_guard_by_type.py::test_g16_resolve_by_type |
4180071.4591602064 iter/sec (stddev: 6.759859410297049e-9) |
4109559.7115422776 iter/sec (stddev: 1.5631666472540378e-8) |
0.98 |
benchmarks/test_guard_by_type.py::test_g17_resolve_by_type_large_registry |
4197045.053660831 iter/sec (stddev: 7.479178082094009e-9) |
4062456.8619314623 iter/sec (stddev: 1.0953092796438548e-8) |
0.97 |
benchmarks/test_guard_cold.py::test_g8_cold_first_resolve |
17949.60354393158 iter/sec (stddev: 0.00006365945555847223) |
15307.93728862876 iter/sec (stddev: 0.0001969289321148439) |
0.85 |
benchmarks/test_guard_cold.py::test_g8b_cold_first_resolve_cached |
15525.598103915205 iter/sec (stddev: 0.00014088923199257765) |
13916.003226467772 iter/sec (stddev: 0.00004747858714939497) |
0.90 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[1] |
535.258617544244 iter/sec (stddev: 0.00038504669193973874) |
559.6367703162622 iter/sec (stddev: 0.00003797737525593629) |
1.05 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[2] |
500.6328252331133 iter/sec (stddev: 0.0002479533759419957) |
507.63280887349197 iter/sec (stddev: 0.0000653548440211759) |
1.01 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[4] |
451.7692715619876 iter/sec (stddev: 0.0002673060263417995) |
453.32872094160234 iter/sec (stddev: 0.000055334667800668593) |
1.00 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[1] |
1694.6139097556918 iter/sec (stddev: 0.00033001276551093516) |
1676.4849308337625 iter/sec (stddev: 0.0001853340875201208) |
0.99 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[2] |
1357.5019818691117 iter/sec (stddev: 0.0003548514365910093) |
1331.2187810426587 iter/sec (stddev: 0.0002111007029352062) |
0.98 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[4] |
1029.9151294139942 iter/sec (stddev: 0.00021246992609297278) |
973.1734993133033 iter/sec (stddev: 0.00022358127729227853) |
0.94 |
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container |
704873.289511182 iter/sec (stddev: 3.4210398510926304e-8) |
714208.3757038184 iter/sec (stddev: 4.789031374137993e-8) |
1.01 |
benchmarks/test_guard_lifecycle.py::test_g6b_build_child_container_auto_scope |
663202.120892823 iter/sec (stddev: 2.9439249284465725e-8) |
673412.6678897241 iter/sec (stddev: 3.458363184597789e-8) |
1.02 |
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle_batch |
2407.554861114934 iter/sec (stddev: 0.000009729119593464122) |
2445.1271038479235 iter/sec (stddev: 0.000013013121771013336) |
1.02 |
benchmarks/test_guard_lifecycle.py::test_g7c_event_loop_floor_control |
61105.18027652289 iter/sec (stddev: 0.0000014163338507618122) |
61631.6177083372 iter/sec (stddev: 0.0000019820789766259074) |
1.01 |
benchmarks/test_guard_lifecycle.py::test_g13_teardown_at_scale |
46792.336747468675 iter/sec (stddev: 0.0000022629695532919364) |
50142.812564184125 iter/sec (stddev: 0.00000204574355320783) |
1.07 |
benchmarks/test_guard_resolve.py::test_g1_transient_resolve |
2318945.8396941866 iter/sec (stddev: 2.5984913848333046e-8) |
2480151.9638407733 iter/sec (stddev: 3.1505248515552476e-8) |
1.07 |
benchmarks/test_guard_resolve.py::test_g2_cached_resolve |
4270451.99533011 iter/sec (stddev: 7.253489909900734e-9) |
4243057.435316781 iter/sec (stddev: 1.2878935598103277e-8) |
0.99 |
benchmarks/test_guard_resolve.py::test_g3_deep_chain |
788986.4591177936 iter/sec (stddev: 4.846522954906891e-8) |
839586.7151415526 iter/sec (stddev: 5.62844635165048e-8) |
1.06 |
benchmarks/test_guard_resolve.py::test_g4_wide_resolve |
466896.05425400194 iter/sec (stddev: 3.942481043165213e-7) |
512694.1665886265 iter/sec (stddev: 3.124843912701075e-7) |
1.10 |
benchmarks/test_guard_resolve.py::test_g5_cross_scope |
1824977.7033438985 iter/sec (stddev: 3.534078440770927e-8) |
1940879.9241168292 iter/sec (stddev: 2.3518279579511867e-8) |
1.06 |
benchmarks/test_guard_resolve.py::test_g9_context_resolve |
1063248.8178956013 iter/sec (stddev: 1.6577266005393405e-7) |
1091855.5410825275 iter/sec (stddev: 1.4674234229418526e-7) |
1.03 |
benchmarks/test_guard_resolve.py::test_g12_override_active_resolve |
790098.1112227896 iter/sec (stddev: 3.966328423046536e-8) |
824907.2155042045 iter/sec (stddev: 5.004985450002708e-8) |
1.04 |
benchmarks/test_guard_resolve.py::test_g18_alias_hop |
2834474.672346339 iter/sec (stddev: 8.966767233033695e-9) |
2853065.8262234754 iter/sec (stddev: 8.824359757850614e-9) |
1.01 |
benchmarks/test_guard_validate.py::test_g10_validate_deep_chain |
26990.725231099546 iter/sec (stddev: 0.000021549215250555847) |
22160.6055617653 iter/sec (stddev: 0.00021837958990580928) |
0.82 |
benchmarks/test_guard_validate.py::test_g11_validate_wide |
14986.567015461966 iter/sec (stddev: 0.00031742112440765965) |
15356.654696171408 iter/sec (stddev: 0.000024849864708945918) |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #472.
Brings every module under
modern_di/to the rule already in AGENTS.md: public API docstrings state the contract, internal helpers get a one-line contract, and each piece of rationale has one home rather than a copy at every site it applies to. No behaviour, no public surface.What went
The comments that cited an invariant test to justify an inlined lookup. The frame-budget rationale they repeated already lives in
test_resolve_costs_exactly_one_resolver_frame_per_node, which is untouched. The heavy narrative docstrings named in the issue — container, wiring, dependency-graph, integrations, scope, and the factory's definition-site paragraph — are cut to what each function guarantees.What stayed
The two comments a reader would otherwise read as a bug:
container.py— the scope map holds ancestors only; ascope: selfentry is a reference cycle.scope.py— the_next_deepermemo is keyed by enum type as well as member, becauseIntEnummembers hash by integer value.Rationale moved rather than deleted
The issue asks for this list, and predicts it will be empty beyond the two above. It is not — four sites carried rationale recorded nowhere else:
__init__, "inlined, not a helper"test_g6_build_child_container_handle_recursion_errorbeing a separate callfind_contextusingin/[]over.get(key, UNSET).getskips a dict subclass's__contains__/__getitem__. No test covers it, and the two-lookup form reads like duplication.container_provider"__init__docstringresolve(Container)works.Test prose
The three provider test modules the issue names no longer describe a per-resolver "override front-guard" or compiled closures; they speak of the compiled constant resolver and the generated template. Test names, parametrizations, assertions and accurate
INVARIANT:docstrings are untouched —arity_rungkeeps its name.Two modules beyond the three are also touched, both for dangling references that #480 created after the issue was written:
test_resolver_compiler.py— the arity comment described a 0/1/generic closure split that the template replaced.test_container.py(4) andtest_context_provider.py(1) — citations ofdocs/adr/2026-*.mdfiles that docs: remove docs/adr; move the decisions that survive to the docs and the code #480 deleted. They now name the section that absorbed the decision, "## 6. Validation is explicit" indocs/introduction/design-decisions.md.ADR-0030 needed no correction. #480 deleted that record;
docs/introduction/design-decisions.mdalready states the free-threaded CI fact the issue asked for.Verifying this is prose-only
Two mechanical checks, both clean:
modern_di/andtests/. This is the check the issue proposes; the script is ~20 lines ofast.dumpwithbody[0]string expressions removed.docs_slugvalues (531 lines) is identical againstmain.just lint-ci,just test-ci(540 passed, 100% line coverage) andjust docs-build(strict) all pass.The number
Prose share of
modern_di/, as a fraction of non-blank lines: 25.4% → 18.6%. Excludingexceptions.py, which this change may not touch: 24.7% → 15.7%.The package figure stays above the 12–15% the issue targets, and I do not think that band is reachable as scoped.
exceptions.pyis 158 prose lines held out of scope — 6.6 points of the package total on its own. Reaching 15% package-wide would mean cutting the rest to ~198 lines, which takes publicContainerdocstrings below the contract the same issue asks them to state. The in-scope figure lands in the band; I would rather report both than hit one by spending the other.