Repository navigation
fix(python): docstrings and __future__ imports are no namespace barriers - #2712
Merged
Merged
Conversation
…space A string-literal statement has no effect and from __future__ import x binds x, but the namespace facts recorded both as unknown statements: a triple-quoted string as an EFFECT, a future import as UNSUPPORTED. sqlalchemy's sql/expression.py lost its 149 re-exports to a docstring after nullsfirst = nulls_first. The extraction test pins every string form (module, class and attribute docstrings, single and triple quotes, r/b/rb/u prefixes, implicit concatenation, an f-string without a replacement field) and the future import forms (alias, parentheses); controls: an f-string with a field (also inside a concatenation), a call, and a star import from __future__ stay barriers. The pipeline fixture adds the expression.py shape end to end and an f-string control package. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The namespace facts proved only plain one-line strings safe, so every triple-quoted string statement (a module, class or attribute docstring) was an unknown EFFECT, and from __future__ import x was UNSUPPORTED. A barrier withdraws every name bound before it: sqlalchemy's sql/expression.py lost its 149 re-exports to the docstring after nullsfirst = nulls_first, and a docstring made an otherwise plain class unproven. A string literal now evaluates nothing in any quoting or prefix (r, b, u, rb), implicit concatenations included; an f-string with a replacement field stays an effect (__format__/__str__ may run code). from __future__ import x [as y] binds like a from-import of module __future__; a star import from __future__ is a syntax error and stays unsupported. CBM_PY_NAMESPACE_FACTS_VERSION moves to 2. The version travels in the lsp_surface py payload and a payload of another version does not decode, so the first incremental run on an index of an older binary falls back to a full re-index (delta.rehydrate_failed -> CBM_PIPELINE_FORCE_FULL_REINDEX) instead of mixing old and new facts. The codec test now spells the current version and rejects a version-1 payload. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The grammar rejects from __future__ import * as Python does, so the file carries an error node and its whole namespace is a PARSE unknown: a stronger barrier than the UNSUPPORTED statement the previous commit guarded for. That guard could never run and is dropped; the control asserts the parse barrier the parser actually yields. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
WHY: the multiple-bindings test pinned an UNKNOWN EFFECT fact for `__all__ = ['escaped\n']`, which only existed because the namespace facts proved plain one-line strings harmless and nothing else. A string literal, escapes included, evaluates nothing, which is this branch's rule, so the fact is gone. ALL_UNKNOWN stays: an escaped name is no literal export name. The assertion still pins the exact stream (13 facts, verified against an index of the same source with the base and this branch). Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…anded) tests/test_pipeline.c conflict: main moved the surface codec to v3 (#2703), this branch moved the Python facts payload to v2; the codec rows carry both (surface v3, py v2), plus the stale py v1 row this branch added. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added a commit
to woodsonl/codebase-memory-mcp
that referenced
this pull request
Oct 20, 2026
…y-namespace-effect-allowlist Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.
What
#2704 publishes a Python package's re-exports from the module's ordered namespace facts. An unknown statement in those facts withdraws every re-export recorded before it. Two kinds of statement were recorded as unknown although they cannot rebind a name:
lib/sqlalchemy/sql/expression.pylost all 149 of its re-exports to the attribute docstrings after them. Only plain one-line strings were proven harmless.This change:
extract_imports.c). This covers any quoting (single, double, triple), ther/b/u/rbprefixes, implicit concatenation, and an f-string without a replacement field, as a module, class or attribute docstring. An f-string with a{...}field stays an effect, alone or inside a concatenation, because its__format__/__str__may run code. The same predicate covers string-literal values, so__all__ = ['escaped\n']is no longer an effect either. Its__all__stays unknown, because an escaped name is not a literal export name.from __future__ import x [as y]binds like a from-import of module__future__. Tree-sitter parses it as its own node type,future_import_statement, without a module field, which is why it was UNSUPPORTED.from __future__ import *is a syntax error in Python and in the grammar, so the file stays a parse unknown.CBM_PY_NAMESPACE_FACTS_VERSION1 → 2. The version travels in thelsp_surface"py" payload. A stored version-1 payload no longer decodes, so the first incremental run on an existing index declines the closure repair (python_namespace_metadata_invalid). It goes through the existingFORCE_FULL_REINDEXpath once: one full re-index, after which the facts are current. Only Python payloads are affected, andSURFACE_CODEC_VERSIONis unchanged. The wire shape is unchanged; version 2 changes which statements extraction proves harmless.Other languages are unchanged.
Known limit:
sql.cast,sql.select,sql.and_lib/sqlalchemy/sql/__init__.py(andsqlalchemy/__init__.py,sqlalchemy/orm/__init__.py) still end with a top-level__go(locals())call. That call is an unknown effect and withdraws those packages' re-exports. Those three names therefore still resolve by the old fallback. Proving that call harmless is a separate change.Evidence
Clean builds (build dir removed before make), par, repeat 2, noise 0 on every repo. Base = #2704's head 67e53d0; head = this branch bf56fcf.
Judged, all 20 moved edges (snippets):
expression.select(...)×2 →sql._selectable_constructors.select(lsp_module_attr); each replaces the guessWriteOnlyCollection.select(suffix_match). Both removals are correct.expression.Join.__init__(...)→selectable.Join.__init__.expression.ClauseList(...)._annotate(...)._set_propagate_attrs(...)→SupportsWrappingAnnotations._annotate,ClauseElement._set_propagate_attrs(MRO).class array(expression.ExpressionClauseList[_T])→ColumnOperators.__getitem__(lsp_operator_dunder). A subscript of a class runs__class_getitem__, not the instance__getitem__. This is an existing operator-dunder shape of the Python LSP, reached now becauseexpression.ExpressionClauseListresolves. It is a separate fix.unique_name/suffix_match(0.38-0.75) →lsp_module_attr/lsp_constructor(0.88-0.92), all throughexpression.<X>. The targets are the savepoint clauses,BindParameter,Grouping,BinaryExpression,ClauseList,_select_iterables,text,type_coerceandnull.Namespace census (the facts as extraction stores them, replayed with #2704's fold):
sql/expression.py, with 149 re-exports. 141 of those names are imported from it elsewhere in the repo (269 importer/name pairs).executor/stats.pygains 2. Nothing imports those names from those modules.Tests
extraction: python_namespace_facts_string_statements_and_future_imports: every string form after an import, the class and attribute docstrings, and three__future__shapes (plain,as, parenthesized). Controls: an f-string field, a concatenated f-string field and a call stay effects;from __future__ import *is a parse unknown.pipeline: pipeline_python_package_reexports_resolve_by_lspadds two cases, each over both drivers × full and incremental: a re-export between docstrings and a__future__import resolves, and one before an f-string with a field does not.pipeline: pipeline_python_namespace_codec_rejects_non_authoritative_rows: a version-1 payload is rejected.extraction: python_namespace_facts_multiple_bindings_and_unsupported_effectsnow pins 13 facts instead of 14. The effect fact for__all__ = ['escaped\n']existed only under the old string rule, and the__all__unknown that follows it is unchanged.__future__statements are unsupported, the version is 1, and the pipeline re-export does not resolve. GREEN with the change.lint-memory-corereports no growth. cppcheck 2.20 (the CI version) and clang-format are clean.