refactor: reconcile invariants and module-size policy - #853
Merged
Conversation
This was referenced Aug 14, 2026
Contributor
Benchmark delta (vs
|
| size | recall@k (head Δ vs base) | MRR (head Δ vs base) | p99 (ms) |
|---|---|---|---|
| 50 | ✅ 0.5649 (+0.0000) | ✅ 0.4978 (+0.0000) | ✅ 0.447 (base 0.650) |
| 83 | ✅ 0.3825 (+0.0000) | ✅ 0.3242 (+0.0000) | ✅ 0.687 (base 1.205) |
| 1000 | ✅ 0.1475 (+0.0000) | ✅ 0.1456 (+0.0000) | ✅ 35.782 (base 47.855) |
Per-backend × per-size matrix
| backend | size | recall@k (Δ) | MRR (Δ) | p99 (ms) |
|---|---|---|---|---|
| bm25 | 100 | ✅ 0.3825 (+0.0000) | ✅ 0.3399 (+0.0000) | ✅ 6.165 (base 8.399) |
| bm25 | 500 | ✅ 0.2250 (+0.0000) | ✅ 0.2165 (+0.0000) | ✅ 28.686 (base 41.865) |
| bm25 | 1000 | ✅ 0.1575 (+0.0000) | ✅ 0.1525 (+0.0000) | ✅ 82.892 (base 112.387) |
| embedding_hashing | 100 | ✅ 0.5175 (+0.0000) | ✅ 0.4360 (+0.0000) | ✅ 8.409 (base 7.218) |
| embedding_hashing | 500 | ✅ 0.2700 (+0.0000) | ✅ 0.2674 (+0.0000) | ✅ 42.768 (base 43.171) |
| embedding_hashing | 1000 | ✅ 0.2000 (+0.0000) | ✅ 0.1931 (+0.0000) | ✅ 96.087 (base 102.979) |
| embedding_st | 100 | skipped (skipped: missing sentence-transformers) | — | — |
| embedding_st | 500 | skipped (skipped: missing sentence-transformers) | — | — |
| embedding_st | 1000 | skipped (skipped: missing sentence-transformers) | — | — |
| fuzzy | 100 | skipped (skipped: missing rapidfuzz) | — | — |
| fuzzy | 500 | skipped (skipped: missing rapidfuzz) | — | — |
| fuzzy | 1000 | skipped (skipped: missing rapidfuzz) | — | — |
| tfidf | 100 | ✅ 0.3825 (+0.0000) | ✅ 0.3220 (+0.0000) | ✅ 0.979 (base 1.193) |
| tfidf | 500 | ✅ 0.2325 (+0.0000) | ✅ 0.2314 (+0.0000) | ✅ 9.186 (base 10.803) |
| tfidf | 1000 | ✅ 0.1475 (+0.0000) | ✅ 0.1456 (+0.0000) | ✅ 35.404 (base 42.986) |
Context pipeline (per scenario)
| scenario | tokens | dropped | dedup |
|---|---|---|---|
| large_catalog | 1480 (base 1480, Δ+0) | 0 (base 0, Δ+0) | 0 (base 0, Δ+0) |
| long_conversation | 2500 (base 2500, Δ+0) | 0 (base 0, Δ+0) | 0 (base 0, Δ+0) |
| mixed_payload | 488 (base 488, Δ+0) | 0 (base 0, Δ+0) | 0 (base 0, Δ+0) |
| short_conversation | 487 (base 487, Δ+0) | 0 (base 0, Δ+0) | 0 (base 0, Δ+0) |
| stress_conversation | 6590 (base 6590, Δ+0) | 11 (base 11, Δ+0) | 4 (base 4, Δ+0) |
| tiny_payload | 256 (base 256, Δ+0) | 0 (base 0, Δ+0) | 0 (base 0, Δ+0) |
Numbers come from make benchmark / make benchmark-matrix.
Latency is hardware-dependent — treat the markers as a rough guide.
See benchmarks/scorecard.md for the full picture.
There was a problem hiding this comment.
Pull request overview
This PR reconciles documented architecture invariants and the module-size policy with the codebase by raising the ordinary module ceiling to 500 lines (while keeping the shrink-only ratchet) and by removing a core→adapters dependency via a shared MCP-result transform placed in a core module.
Changes:
- Raised the mechanically enforced module-size limit to 500 lines, re-froze the baseline, and updated the checker/tests/docs accordingly.
- Folded previously split “helper” modules back into their natural owners (
ScoringConfigintoconfig.py, consolidation helpers intocontext/consolidation.py). - Moved the MCP result→envelope transform into
src/contextweaver/_mcp_result.py, re-exported it from the MCP adapter, and added an architecture regression test for the context-layer boundary.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_layer_boundaries.py | Adds AST-based regression tests to prevent context/ importing contextweaver.adapters and to preserve the adapter import path for the MCP transform. |
| tests/test_check_module_size.py | Updates module-size checker tests to derive synthetic sizes from the configured LIMIT. |
| src/contextweaver/context/ingest.py | Switches MCP ingestion to use the new core _mcp_result transform and updates surrounding comments/docstrings. |
| src/contextweaver/context/consolidation.py | Inlines consolidation helper functions back into the main consolidation module under the new size ceiling. |
| src/contextweaver/context/_consolidation_helpers.py | Removes the now-unneeded consolidation helper module (folded into consolidation.py). |
| src/contextweaver/config.py | Moves ScoringConfig back into config.py and updates related docstrings. |
| src/contextweaver/adapters/mcp.py | Re-exports mcp_result_to_envelope from the new core module and trims internal duplication. |
| src/contextweaver/_scoring_config.py | Removes the now-unneeded scoring config helper module (folded into config.py). |
| src/contextweaver/_mcp_result.py | Introduces the shared, pure MCP result dict → ResultEnvelope transform below the adapter layer. |
| scripts/module_size_baseline.json | Updates the frozen ceilings list to reflect the new 500-line ordinary limit (keeping >500 modules frozen). |
| scripts/check_module_size.py | Raises LIMIT to 500 and updates messaging/docs in the checker script. |
| llms-full.txt | Regenerates the generated agent guidance artifact after invariant/doc updates. |
| docs/agent-context/invariants.md | Updates invariant wording to separate mechanically enforced invariants vs review policy and reflects new module-size policy. |
| AGENTS.md | Updates the canonical module map and module-size guidance to match the new policy and folded modules. |
| .claude/CLAUDE.md | Aligns Claude-specific guidance with canonical docs and removes duplicated/fragile statements. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+13
to
+22
| def _imported_modules(path: Path) -> list[str]: | ||
| """Return absolute module names imported by *path*.""" | ||
| tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) | ||
| modules: list[str] = [] | ||
| for node in ast.walk(tree): | ||
| if isinstance(node, ast.Import): | ||
| modules.extend(alias.name for alias in node.names) | ||
| elif isinstance(node, ast.ImportFrom) and node.module: | ||
| modules.append(node.module) | ||
| return modules |
| from contextweaver.envelope import ResultEnvelope | ||
| from contextweaver.types import ArtifactRef | ||
|
|
||
| logger = logging.getLogger("contextweaver.adapters") |
Comment on lines
+150
to
+151
| **Enforcement:** currently **review policy** pending the import-linter work in | ||
| #648. Changes fixing #752 should add a focused regression check where practical. |
| Returns: | ||
| Promoted facts, ordered by ``fact_id`` for determinism. | ||
| """ | ||
| """Promote qualifying *clusters* into :class:`PromotedFact` records (#680).""" |
| Raises: | ||
| ConfigError: If *policy* fails validation. | ||
| """ | ||
| """Run the consolidation pipeline over *episodic_store* (issue #498).""" |
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
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.
Summary
Implements the maintainer decisions for #752 and #753 as one architectural change-set because both issues govern the same module-structure and agent-guidance surface.
#753 — module-size policy
LIMIT;_scoring_config.py→config.py;context/_consolidation_helpers.py→context/consolidation.py;AGENTS.md, including helper-module rationales, module-size commands, and the obsolete mixin rationale.#752 — invariant reconciliation
tool_idround-trip to MCP-facing surfaces that adopt the canonical contract; framework-local IDs remain an explicit loose class;ContextManager's public API/behavior the invariant and treats mixin composition as private implementation detail;context -> adaptersdependency: the pure MCP-result transform now lives in neutral_mcp_result.py,context/ingest.pyconsumes it directly, andadapters.mcpre-exports it for compatibility;context/importscontextweaver.adapters;.claude/CLAUDE.mdand the canonical agent guidance to match shipped architecture.Generated documentation
llms-full.txtwas regenerated using the repository's canonicalscripts/gen_llms.pyafter the agent guidance changed. The generated-artifact drift gate passes on the final head.Validation
Final GitHub Actions state is fully green:
Closes #752
Closes #753
Refs #456 #648 #743