Repository navigation
feat(sync): add the Python adapter - #991
Conversation
794bcb2 to
07ab9e4
Compare
9b0dc0f to
b2e309e
Compare
ctawiah
changed the title
b2e309e to
aaeb409
Compare
eb1bb51 to
3deaad9
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| remaining-- | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
Defaults use function-body scoping
Low Severity
Parameter defaults sit under function_definition, so pythonDeferredUse and pythonScopedName treat them like the function body. Python evaluates defaults at definition time in the enclosing scope, so a default can pick up a later module constant, miss an earlier class attribute, or get hidden by a parameter of the same name.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 995ab51. Configure here.
There was a problem hiding this comment.
Fixed. pythonScopeChain and pythonDeferredUse skip a function for a node in a parameter default, through pythonInParameterDefault, so a default runs in the enclosing scope at definition time: it reads a module constant in module scope, misses a later constant, and reads a class attribute in a method. TestPythonParameterDefaultsRunInTheEnclosingScope covers the three cases.
| }) | ||
| for id := range ambiguous { | ||
| delete(instances, id) | ||
| } |
There was a problem hiding this comment.
Non-framework assignments drop client kinds
High Severity
pythonInstances treats any kind change as ambiguous, including an empty kind from self.client = None or client = mock(). Combined with class-wide self.client ids, a cleanup or mock assignment in any method deletes the client kind for the whole class, so chat.completions.create and messages.create calls are not collected.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 84c14fb. Configure here.
There was a problem hiding this comment.
Fixed in part, by design. A reset such as self.client = None in a cleanup does not change the kind of the client, so the instance walks skip a None, null, or undefined value and the requests of the class are collected. A mock assigned to the same name still hides the client, because the existing behavior and its test treat a mock as a value of another kind, and a static reader cannot tell which assignment runs. TestPythonClientResetsDoNotDropTheClientKind and TestTypeScriptClassFieldsDoNotHideImports cover a reset next to a request.
| ordinals := map[string]int{} | ||
| code.Walk(tree.RootNode(), func(node *sitter.Node) { | ||
| if node.Kind() != "assignment" { | ||
| return |
There was a problem hiding this comment.
Walrus bindings skip named resources
Medium Severity
Resource collection and pythonInstances only walk assignment nodes, not named_expression. A walrus such as agent := Agent(...) or client := OpenAI() is not named after the variable and is not recorded as a client instance, so follow-on client.chat.completions.create calls are missed. Constants already handle the walrus via pythonDeclaredVariable.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 84c14fb. Configure here.
There was a problem hiding this comment.
Fixed. A walrus is an assignment for the resource walk, the instance walk, and the helpers that tell an assigned call from an unassigned one, through pythonAssignmentParts, so agent := Agent(...) names the agent after the variable and client := OpenAI() is a client. TestPythonWalrusBindingsNameResourcesAndClients covers both.
| // a subscript, `items[0] = b = Agent()`, names nothing, so the inner | ||
| // name collects them. | ||
| if pythonNamedAssignment(node.Parent()) != nil { | ||
| return |
There was a problem hiding this comment.
Walrus calls skipped by both walks
Medium Severity
pythonAssignmentTarget treats any identifier assignment in the parent chain as a value chain, so a walrus like x = [a := Agent()] is skipped as an “inner link.” The call walk then also skips it because pythonAssignedValue sees the walrus, and the agent is never collected.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 43573d3. Configure here.
There was a problem hiding this comment.
Fixed. The inner-link check follows the assignment chain only, through pythonChainTarget: an outer assignment is a link when it holds the inner one as its whole value, so a walrus inside a list or a call, as in x = [a := Agent()], names its own call. TestPythonWalrusBindingsNameResourcesAndClients covers a walrus inside a list next to an unassigned call.
| for parent := node.Parent(); parent != nil; parent = parent.Parent() { | ||
| switch parent.Kind() { | ||
| case "lambda": | ||
| return true |
There was a problem hiding this comment.
Lambda defaults resolve as runtime uses
Medium Severity
pythonDeferredUse treats every name inside a lambda as running when the lambda is called. Python evaluates a lambda’s default arguments at definition time, like a function default, so those names should use the enclosing scope at that point. A default can pick up a later constant or import, or miss an unbound local that would fail at definition, and pythonInParameterDefault only covers function_definition.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit d87328a. Configure here.
There was a problem hiding this comment.
Fixed. pythonInParameterDefault covers a lambda too, so pythonDeferredUse and the alias visibility treat a default of a lambda as code that runs with the definition, in the enclosing scope at that point. TestPythonLambdaDefaultsRunAtDefinition covers a default before a later constant and one after an earlier constant.
| if attribute, _ := pythonChainRoot(rest); (rootName == "self" || rootName == "cls") && attribute != "" { | ||
| return strings.Join(append(pythonClassScopes(node, source), attribute), ".") | ||
| } | ||
| return pythonScopedName(node, rootName, source, declared) |
There was a problem hiding this comment.
Class-name mutations skip class attributes
Medium Severity
pythonMutatedName only maps self.attr and cls.attr writes onto the class attribute. A mutation through the class name, such as Bot.items.append(...) or Bot.items[0] = ..., scopes only the root Bot, so Bot.items stays a writable constant after an in-place change.
Reviewed by Cursor Bugbot for commit d87328a. Configure here.
There was a problem hiding this comment.
Fixed. pythonMutatedName maps a write through the class name, such as Bot.items.append(...) or Bot.names[0] = ..., onto the class attribute when the file declares it, so the attribute is read-only after an in-place change. TestPythonClassNameMutationsFreezeTheClassAttribute covers a mutating call and a subscript write.
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Lambda parameters hide enclosing names
Medium Severity
Lambda parameters are recorded with pythonQualifiedID, whose scope chain only includes function_definition and class_definition. A parameter of a nested lambda is therefore treated as a local of the enclosing function. That pre-binds the name for the whole function and hides a module constant of the same name, so a later agent or prompt in that function reads as dynamic even though Python only binds the name inside the lambda.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit c9b6f69. Configure here.
There was a problem hiding this comment.
Not reproducible. The parameter walk matches the parameters node of a def, and tree-sitter gives a lambda a lambda_parameters node, so a lambda parameter is never recorded as a local of the enclosing function. pythonLocalBinding binds it inside the lambda only. TestPythonLambdaParametersBindOnlyInsideTheLambda pins this: a lambda body reads its parameter, and an agent after the lambda in the same function still reads the module constant.
| if alias := value.ChildByFieldName("alias"); alias != nil { | ||
| return []*sitter.Node{alias} | ||
| } | ||
| } |
There was a problem hiding this comment.
Except aliases never bind names
Medium Severity
pythonBindingTargets handles except_clause like with_item, looking for a value field whose kind is as_pattern. Tree-sitter's except_clause has no value field, so except E as name never records a binding. A module constant of that name stays writable through the handler, and a write would edit the constant instead of treating the alias as dynamic.
Reviewed by Cursor Bugbot for commit c9b6f69. Configure here.
There was a problem hiding this comment.
Not reproducible. In the tree-sitter Python grammar that sync vendors (v0.25.0), except E as name parses as (except_clause value: (as_pattern ... alias: (as_pattern_target (identifier)))), so the value branch binds the alias. TestPythonExceptAliasesHideTheOuterConstant pins this at module level and in a function: an agent that reads the alias, inside or after the handler, is read-only.
| frozen := value.Clone() | ||
| frozen.MarkReadOnly("mutated constants are read-only") | ||
| environment[name], values[name] = frozen, frozen | ||
| } |
There was a problem hiding this comment.
Aliased imports ignore sibling mutations
Medium Severity
Mutation tracking freezes only the binding that was written. Two aliases of the same import, such as TEXT and ALSO, share one object, but TEXT.append(...) leaves ALSO writable with the pre-mutation value, so a publish can emit messages or constants that the file already changed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b0cf4eb. Configure here.
There was a problem hiding this comment.
Fixed. pythonSpreadMutations links the names that share one object, two imports that bind one value of another file and a declaration that binds a bare name to another constant, and spreads a mutation to every linked name, so ITEMS.append(...) freezes ALSO too, and a mutation through a local alias freezes its origin. TestPythonMutationThroughAnAliasFreezesEveryName covers both, with an untouched import that stays writable.
| } | ||
| id := pythonQualifiedID(node, code.Text(name, source), source) | ||
| declarations = append(declarations, declaration{node: node, id: id, value: valueNode}) | ||
| counts[id]++ |
There was a problem hiding this comment.
Global assignment skips module mutation
Medium Severity
A global or nonlocal assignment is recorded as a function-local declaration through pythonQualifiedID, so it does not freeze the module (or enclosing) constant. Sync can still treat that constant as a direct value and write the original literal after the function has rebound it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b0cf4eb. Configure here.
There was a problem hiding this comment.
Fixed. Declarations and binding targets take their id from pythonBindingID, which moves a name that the function declares global to the module and a name declared nonlocal to the nearest enclosing function, so the outer constant counts as reassigned and reads as read-only, and no local of the function is recorded. TestPythonGlobalAndNonlocalAssignmentsRebindTheOuterConstant covers a global, a nonlocal, and a plain local that still shadows.
| if id := pythonMutatedName(node, owner, source, declared); id != "" { | ||
| mutated[id] = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Delete statements skip mutation tracking
Low Severity
The mutation walk handles assignments, walruses, and in-place methods, but not delete_statement. del ITEMS[0] or del MESSAGES[-1] leaves that collection writable, so a write-back can restore entries the source already removed.
Reviewed by Cursor Bugbot for commit b0cf4eb. Configure here.
There was a problem hiding this comment.
Fixed. The mutation walk handles delete_statement: del ITEMS[0], del CONFIG["key"], PAIR[1], and del NAME mark their root constant mutated, so the constant is read-only and a write-back cannot restore a removed entry. TestPythonDeleteStatementsFreezeTheCollection covers a subscript, an attribute list, and a bare name, with an untouched list that stays writable.
| // name collects them. | ||
| if pythonNamedAssignment(node.Parent()) != nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
Walrus prompts keep the outer name
Medium Severity
The prompt inner-link skip still uses pythonNamedAssignment, so any enclosing named assignment counts as a chain. A walrus prompt inside a list or call is collected under the outer variable instead of its own name. Framework calls already use pythonChainTarget for this, which only follows an outer assignment when the inner node is its whole value.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 197b4df. Configure here.
There was a problem hiding this comment.
Fixed. The prompt walk skips an assignment only when it is a link of a chain, through pythonChainTarget, as the framework calls already did, and pythonPromptCalls leaves a call that a walrus inside the value binds to that name, so batch = [reply := client.chat.completions.create(...)] names the prompt reply once. TestPythonWalrusPromptsKeepTheirOwnName covers a walrus in a list and a chain.
| if messages := code.MessageList(constant); messages != nil { | ||
| return messages | ||
| } | ||
| } |
There was a problem hiding this comment.
Messages skip inner local bindings
Medium Severity
pythonMessages resolves a bare messages name through pythonConstant only, so it never applies pythonLocalBinding. A lambda parameter or comprehension target that reuses a module messages name is treated as that outer list, which is then writable.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3e6c2d5. Configure here.
There was a problem hiding this comment.
Fixed. pythonMessages checks pythonLocalBinding before it resolves a bare name, so a lambda parameter or a comprehension target that reuses a module messages name is read-only instead of the outer list. TestPythonMessagesSkipInnerLocalBindings covers a lambda parameter, a comprehension target, and a plain use that still resolves.
| if pythonParameterName(parameter, source) == name { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
Lambda defaults bind inner parameters
Medium Severity
pythonLocalBinding treats any use inside a lambda as a parameter when the name matches, including default values. Defaults run in the enclosing scope, so a default that shares a parameter name is marked dynamic instead of resolving the outer constant.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3e6c2d5. Configure here.
There was a problem hiding this comment.
Fixed. pythonLocalBinding skips a lambda when the use sits in one of its parameter defaults, and skips a comprehension when the use sits in the iterable of its first for clause, because both run in the enclosing scope. TestPythonLambdaDefaultsAndFirstIterablesSeeTheOuterName covers a default and a first iterable that resolve the module constant, next to a lambda body and a comprehension body that read their own binding.
| value := constants[binding.name] | ||
| if value == nil { | ||
| value = code.UnresolvedImport(binding.module, binding.name, document.Path) | ||
| } |
There was a problem hiding this comment.
Imported mutated names look unresolved
Medium Severity
A dependency that reassigns or mutates a name stores nil under that id, so a later from … import of it becomes UnresolvedImport with ErrorImportUnresolved. The linked file still looks missing, and the initial literal is dropped instead of staying a read-only binding.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit dc79b19. Configure here.
There was a problem hiding this comment.
Fixed. A file keeps the first literal of a name that it reassigns or mutates under code.RecordFrozen, and an import of that name binds the literal as read-only with the reason of the file, so the import resolves and blocks a write instead of reading as unresolved. The TypeScript adapter had the same gap and uses the same record. TestPythonImportsOfReassignedAndMutatedNamesAreReadOnlyBindings and TestTypeScriptImportsOfReassignedNamesAreReadOnlyBindings cover a reassigned string and a mutated list next to a plain import.
| for _, scope := range scopes[:depth+1] { | ||
| names = append(names, scope.name) | ||
| } | ||
| return strings.Join(append(names, name), ".") |
There was a problem hiding this comment.
Nonlocal skips the wrong function
Low Severity
A nonlocal assignment is rebound on the nearest enclosing function, even when that function does not declare the name. Python binds it on the nearest enclosing function that already has the name, so an intermediate empty function can hide the real reassignment and leave the outer constant writable.
Reviewed by Cursor Bugbot for commit dc79b19. Configure here.
There was a problem hiding this comment.
Fixed. In nonlocal mode pythonBindingID skips an enclosing function that does not bind the name itself, through pythonFunctionBinds (parameters and the assignment, walrus, loop, with, except, and unpacking targets of its own body), so the rebinding lands on the nearest function that has the name. TestPythonNonlocalBindsTheNearestFunctionThatHasTheName covers an empty middle function.
| counts[id]++ | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Defs and classes skip name bindings
Medium Severity
pythonConstants records assignments, parameters, and for/with/except/case targets, but a def or class never binds its name. A later function or class with the same name as a constant still resolves to the earlier literal, so an agent can look writable with the wrong instructions after that name is rebound. pythonFunctionBinds has the same gap, so nonlocal can also skip the function that actually owns the name.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit cfff96a. Configure here.
There was a problem hiding this comment.
Fixed. A def or a class binds its name in the enclosing scope as a declaration without a value, so a later definition with the name of a constant makes the constant read-only and hides it from there on, and pythonFunctionBinds counts a nested def or class, so a nonlocal lands on the function that owns the name through a definition. TestPythonDefsAndClassesBindTheirNames covers a def, a class, and a nonlocal through a def.
| value := *wildcard | ||
| value.Reason = fmt.Sprintf("star import cannot resolve %s statically", code.Text(node, source)) | ||
| return &value | ||
| } |
There was a problem hiding this comment.
Star import shadows function locals
Medium Severity
After pythonConstant misses a name, identifier lookup always falls through to the star-import wildcard. A function parameter or non-literal local already has a nil constants entry, so that binding is reported as an unresolved import (with the wildcard’s file and 0,0 span) instead of a dynamic local. Sync then hints about linking a dependency for a name that is local to the function.
Reviewed by Cursor Bugbot for commit 47ad600. Configure here.
There was a problem hiding this comment.
Fixed. Before the star-import fallback, the identifier lookup checks the scoped name: an entry without a value, which a parameter, a target, or a binding without a literal leaves, makes the name a dynamic local (dynamic expression has no safe inverse, no import code), and only a name with no binding in scope falls through to the wildcard. TestPythonStarImportDoesNotSupplyFunctionLocals covers a parameter, a dynamic local, and a name that the star import does supply.
| } | ||
| } | ||
| visit(function.ChildByFieldName("body")) | ||
| return bound |
There was a problem hiding this comment.
Nonlocal skips imported enclosing names
Medium Severity
pythonFunctionBinds treats parameters, assignments, loops, with/except aliases, and unpacking as bindings, but not from … import (or import) names. nonlocal therefore skips an enclosing function that only imported the name and rebinds the module constant instead, so a nested assignment can freeze or overwrite the wrong binding.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 47ad600. Configure here.
There was a problem hiding this comment.
Fixed. pythonFunctionBinds counts the names that an import or a from ... import statement binds in the function, through pythonImportedNames, so a nonlocal lands on the function that imported the name instead of the module constant. A global or nonlocal assignment also counts as a rebinding of its target, because it runs when its function runs, so the target is read-only even when it is the only assignment. TestPythonNonlocalBindsAnImportedEnclosingName covers a function-local import with a nonlocal in a nested function, and checks that the module constant stays writable.
| return candidate | ||
| } | ||
| } | ||
| return name |
There was a problem hiding this comment.
Nested global skips the wrong name
Medium Severity
pythonBindingID honors global and nonlocal when recording assignments, but reads and mutations resolve through pythonScopedName, which only walks enclosing declarations. In a nested function that declares global NAME while an outer function also binds NAME, a use or NAME.append(...) still attaches to the outer local instead of the module name, so the wrong constant is read and the actually mutated object can stay writable.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 8e15573. Configure here.
There was a problem hiding this comment.
Fixed. pythonScopedName asks pythonDeclaredScopeID first: when the innermost function declares the name global or nonlocal, a read or a mutation resolves to the scope that pythonBindingID gives the writes, so NAME.append(...) under global NAME freezes the module constant and a read skips the local of the enclosing function. TestPythonGlobalStatementsScopeReadsAndMutations covers both next to an untouched outer local.
| for _, name := range names[1:] { | ||
| link(names[0], name) | ||
| } | ||
| } |
There was a problem hiding this comment.
Aliased imports miss mutation spread
Medium Severity
pythonSpreadMutations treats two imports as the same object only when they share a pointer. An alias in the dependency such as ALSO = ITEMS is stored as a Clone(), so importing both names yields two nodes that still point at one literal. Mutating one freezes only that name; the alias stays writable and a write-back can overwrite the mutated collection.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 8e15573. Configure here.
There was a problem hiding this comment.
Fixed. A file records ALSO = ITEMS as an alias through code.RecordAlias when the alias is assigned once, and an import resolves the alias to its origin through code.AliasOrigin, so both names import one value and pythonSpreadMutations links them by pointer. The TypeScript adapter does the same for export const ALSO = ITEMS. TestPythonImportedAliasesShareMutations and TestTypeScriptImportedAliasesShareMutations cover a mutation through the origin that freezes the alias.
| } else { | ||
| switch pythonScopeStatement(parent, name, source) { | ||
| case "global_statement": | ||
| return name |
There was a problem hiding this comment.
Nonlocal search follows enclosing global
Medium Severity
When a nested function has nonlocal for a name, pythonBindingID still returns on an intervening function's global statement and treats the write as module-level. That global only affects that middle function, so the search should skip it and keep looking for the nearest enclosing local binding. Reads and mutations that go through pythonDeclaredScopeID follow the same path, so the wrong constant can be frozen or selected for a write.
Reviewed by Cursor Bugbot for commit aabcf69. Configure here.
There was a problem hiding this comment.
Fixed. In nonlocal mode pythonBindingID skips a function between that declares the name global, because that statement binds the name for that function only and leaves it without a local, so the search continues to the nearest enclosing function that binds the name. Reads and mutations through pythonDeclaredScopeID follow the same path. TestPythonNonlocalSkipsAGlobalOfAFunctionBetween covers a global in the function between and checks that the module constant stays.
| case "assignment", "augmented_assignment", "named_expression": | ||
| if left, _ := pythonAssignmentParts(node); left != nil && left.Kind() == "identifier" && code.Text(left, source) == name { | ||
| bound = true | ||
| return |
There was a problem hiding this comment.
Augmented assignment skipped for nonlocal
Medium Severity
pythonFunctionBinds lists augmented_assignment as a local binding, but it asks pythonAssignmentParts for the target, and that helper only handles assignment and named_expression. A function whose only write to the name is += (or another augmented op) is treated as not binding it, so a nested nonlocal skips that function and lands on the wrong scope.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit aabcf69. Configure here.
There was a problem hiding this comment.
Fixed. pythonFunctionBinds reads the target of an augmented_assignment from its left field, so a function whose only write is NAME += value binds the name and a nested nonlocal lands on it. TestPythonAugmentedAssignmentsBindInTheirFunction covers a parameter with an augmented write and a nonlocal below it.
| // The nil entries shadow an import of the same name here and in | ||
| // every later literal. | ||
| values[item.id], environment[item.id] = nil, nil | ||
| continue |
There was a problem hiding this comment.
Mutated aliases skip frozen records
Medium Severity
ALSO = ITEMS is meant to record an alias and freeze both names when ITEMS is mutated, but mutated names never enter environment, so the alias evaluates as non-literal and the loop continues before RecordAlias or RecordFrozen. An import of ALSO from that file then reads as unresolved instead of a read-only copy of the origin.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a04cc4c. Configure here.
There was a problem hiding this comment.
Fixed. The alias record of ALSO = ITEMS comes before the literal check, so a mutated or reassigned origin, which has no literal in the environment, still records the alias, and an import of the alias resolves through code.AliasOrigin to the frozen first literal of the origin as read-only. TestPythonImportOfAnAliasOfAMutatedNameIsReadOnly covers an alias of a mutated list and an alias of a reassigned string.
| code.Walk(root, func(node *sitter.Node) { | ||
| name, valueNode := pythonDeclaredVariable(node) | ||
| if name == nil || valueNode == nil { | ||
| return |
There was a problem hiding this comment.
Bare annotations fail to shadow
Medium Severity
A function annotation with no value (TEXT: str) makes that name local in Python, but pythonDeclaredVariable drops assignments whose right-hand side is missing, and identifier targets are not recorded as binding targets either. Uses of that name still resolve to the outer constant, so a prompt write can edit the module value the function does not actually read.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a04cc4c. Configure here.
There was a problem hiding this comment.
Fixed. A bare annotation, TEXT: str, binds its name in its scope as a declaration without a value, so a use in the function is dynamic instead of the module constant. An assignment of the name is the binding when there is one, so the annotation does not count as a reassignment. TestPythonBareAnnotationsBindTheirName covers an annotation alone, an annotation with a later assignment, and the module constant that stays.
| if alias := value.ChildByFieldName("alias"); alias != nil { | ||
| return []*sitter.Node{alias} | ||
| } | ||
| } |
There was a problem hiding this comment.
Except-star aliases are not bound
Low Severity
Binding collection handles except_clause aliases but not except_group_clause (except*). A name bound in except* ExceptionGroup as err never shadows an outer constant, so later reads can still resolve and write that constant.
Reviewed by Cursor Bugbot for commit a04cc4c. Configure here.
There was a problem hiding this comment.
The grammar that sync vendors parses except* ExceptionGroup as err as an except_clause, so the alias was already bound. The binding targets now also accept an except_group_clause, for a grammar that gives the group clause a kind of its own. TestPythonExceptGroupAliasesBindTheirName covers an except* alias that hides a module constant.
| } | ||
| if id := pythonMutatedName(node, code.Text(target, source), source, declared); id != "" { | ||
| mutated[id] = true | ||
| } |
There was a problem hiding this comment.
Function += and del freeze outer names
Medium Severity
pythonDeclaredVariable only treats plain assignment and walrus as declarations, so a function-local NAME += … or del NAME is not recorded as a local binding. The mutation walk then attributes that write to the enclosing constant through pythonMutatedName. Python treats both as locals of the whole function (UnboundLocalError without global); the adapter instead freezes the module (or outer) constant and blocks writes to it. pythonFunctionBinds already counts += for nonlocal, so this is incomplete for reads and mutations.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit edee320. Configure here.
There was a problem hiding this comment.
Fixed. In a function, NAME += value and del NAME bind the name as a local when the function binds it in no other way, so the write hides the outer constant instead of freezing it, and a function that already binds the name, through an import, an assignment, or a parameter, has that local mutated. A global statement keeps the write on the module constant, and at module level and in a class body the write still mutates the constant of that scope. TestPythonAugmentedAndDeleteWritesInAFunctionBindLocals covers an augmented write, a delete, and a delete under a global statement.
| return value.Clone() | ||
| } | ||
| return code.ShadowedImport(constants, scoped, use) | ||
| } |
There was a problem hiding this comment.
Class body misses earlier outer names
Medium Severity
pythonScopedName prefers a class attribute whenever the class binds that name anywhere, and pythonConstant only falls back to a shadowed import when the use sits before that assignment. Class bodies run in order and should still see the enclosing module constant until the class binds the name. A resource in the class body that reads the name before the class assignment therefore becomes unresolved instead of using the module value. Imports are handled; ordinary module constants are not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit edee320. Configure here.
There was a problem hiding this comment.
Fixed. pythonConstantWith resolves a use in a class body that comes before the class binds the name through the ids without that class attribute, so the use reads the enclosing module constant, as Python does in a class body that runs in order. A use after the class assignment reads the class attribute. TestPythonClassBodiesSeeTheModuleConstantBeforeTheClassBindsIt covers both.
| return pythonConstantWith(node, name, use, source, constants, except) | ||
| } | ||
| } | ||
| return code.ShadowedImport(constants, scoped, use) |
There was a problem hiding this comment.
Class body skips outer name
Medium Severity
pythonConstantWith returns as soon as the class-scoped entry is nil, so it never runs the later fallback that should read the enclosing name. A class-body use of TEXT before a later class binding, or beside a non-literal/reassigned class TEXT, does not see the module constant even though that is how Python resolves it.
Reviewed by Cursor Bugbot for commit f66f19c. Configure here.
There was a problem hiding this comment.
Fixed. The constants record where the first declaration of each name starts, and pythonConstantWith falls back to the enclosing name when a class-body use comes before the class binds the name at all, before it looks at the class entry, so a later literal, a reassigned attribute, and a dynamic attribute all leave the earlier use to the module constant. TestPythonClassBodiesSeeTheModuleConstantBeforeTheClassBindsIt covers the three cases.
| declarations = append(declarations, declaration{node: node, id: id, value: nil}) | ||
| counts[id]++ | ||
| } | ||
| } |
There was a problem hiding this comment.
Bare annotations bind in class bodies
Medium Severity
A bare annotation is recorded as a binding in every scope, including class and module bodies. In those scopes it only fills __annotations__ and does not hide an outer name, so a later use can miss the module constant and become read-only. Function bodies are the case where this hiding is correct.
Reviewed by Cursor Bugbot for commit 1127d9d. Configure here.
There was a problem hiding this comment.
Fixed. A bare annotation binds its name only in a function body, where Python makes the name local. In a module or a class body the annotation fills the annotations and binds nothing, so a later use still reads the module constant. TestPythonBareAnnotationsBindTheirName now covers a class body and a module annotation that bind nothing next to the function cases.
| origin := pythonScopedName(item.value, code.Text(item.value, source), source, declared) | ||
| if origin != item.id && declared(origin) { | ||
| link(item.id, origin) | ||
| } |
There was a problem hiding this comment.
Alias mutations ignore later reassignment
Medium Severity
pythonSpreadMutations links every identifier assignment to its origin, including names that are later reassigned. A mutation of one origin then freezes unrelated constants that only shared the name after a later assignment. Alias recording already requires a single unmoved assignment.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1127d9d. Configure here.
There was a problem hiding this comment.
Fixed. pythonSpreadMutations links an identifier assignment to its origin only when the name is assigned once and not moved by a global or nonlocal statement, the same rule as the alias record, so a name that is assigned again does not spread a mutation to the constant it held first. TestPythonReassignedAliasesDoNotSpreadMutations covers an alias that is assigned again and then mutated.
| // `a = b = "text"` nests the second assignment as the value of the first. | ||
| if right := node.ChildByFieldName("right"); right != nil { | ||
| return pythonValue(right, source, constants) | ||
| } |
There was a problem hiding this comment.
Walrus values skipped in constant reads
Medium Severity
pythonValue follows parenthesized expressions and chained = assignments but does not follow a named_expression. A walrus on the right of an assignment or inside a list or dict therefore reads as dynamic, so static literals such as NAME = (OTHER := "text") and [x := "hello", "world"] become read-only instead of writable constants. Resource collection already unwraps this form through pythonUnwrap.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 5f1dd05. Configure here.
There was a problem hiding this comment.
Fixed. pythonValue follows a named_expression to its value, as the resource collection does, so NAME = (OTHER := "text") and [x := "hello", "world"] read as writable constants. TestPythonWalrusValuesReadAsConstants covers a walrus as the value of an assignment and a walrus inside a list.
Add the shared tree-sitter helpers, string codecs, and placeholder translation. Add the Python adapter: it finds agents, prompts, tools, and constants through an import allowlist, follows static repository imports, and never runs Python. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 37 total unresolved issues (including 36 from previous reviews).
Reviewed by Cursor Bugbot for commit aaaccc6. Configure here.
| fields["content"] = adapter.ReadOnly(int(node.StartByte()), int(node.EndByte()), "message content is not static") | ||
| } | ||
| return &adapter.Node{Fields: fields} | ||
| } |
There was a problem hiding this comment.
Named messages in lists not resolved
Medium Severity
pythonMessage only reads inline tuples, dicts, and constructors, so a literal message list that names a constant such as from_messages([SYSTEM, HUMAN]) never loads that constant. pythonValue and a whole-list identifier already follow those names, so this form becomes read-only instead of a writable prompt.
Reviewed by Cursor Bugbot for commit aaaccc6. Configure here.
Context
Python sources need static inspection of agents, prompts, tools, and constants, with literal edits and placeholder translation.
What changes
Design notes
Review focus
Verification
make buildgo test ./...golangci-lint run ./...Related changes
🤖 Generated with Claude Code
Note
Overview
Adds tree-sitter dependencies and a new Python sync adapter that statically inspects
.pysources (no Python execution) to project agents, prompts, tools, and constants for LaunchDarkly sync.Introduces the shared
adapter/codepackage for bounded parsing, Python/JS string codecs, message lists,{{name}}placeholder translation, import/constant helpers, and positional resource IDs.adapter.DocumentgainsReadOnlyso values from generated or vendored dependency files stay non-writable.The Python adapter uses a framework import allowlist and call rules to recognize LangChain, CrewAI, OpenAI clients, and similar patterns; it follows static repository imports, marks dynamic, mutated, or ambiguous values read-only, and encodes writes as Python literals.
resolve.gorecords positional id selections with sibling-aware identity so index-based mappings stay stable when unnamed resources shift.Reviewed by Cursor Bugbot for commit aaaccc6. Bugbot is set up for automated code reviews on this repo. Configure here.