Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds script/core/diagnostics/devsper-schema.lua which fires warnings on .devsper files for missing required spec fields (prompt, source, run, description) and unknown builtin: plugin names. Registered as the 'devsper-schema' diagnostic in the 'devsper' group via proto/diagnostic.lua.
…yield Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces language support for the Devsper workflow engine, including VS Code extension assets, Lua meta-definitions, and custom diagnostics for the Lua Language Server. The review identified several issues in the diagnostic implementation: a correctness bug in devsper-depends.lua where AST nodes were incorrectly accessed, performance inefficiencies due to redundant sorting and multiple AST traversals, and brittle logic in isLocalMethod that relies on specific variable naming rather than the VM's type system.
| -- Pass 2: check depends_on entries in each wf.task spec | ||
| guide.eachSourceType(state.ast, 'call', function (source) | ||
| await.delay() | ||
| if not isLocalMethod(source, 'wf', 'task') then return end | ||
| local args = source.args | ||
| if not args then return end | ||
| local spec = args[2] | ||
| if not spec or spec.type ~= 'table' then return end | ||
|
|
||
| local dependsOn = tableGet(spec, 'depends_on') | ||
| if not dependsOn or dependsOn.type ~= 'table' then return end | ||
|
|
||
| -- Build sorted list of declared IDs for error messages | ||
| local sortedIds = {} | ||
| for k in pairs(declaredIds) do | ||
| sortedIds[#sortedIds + 1] = k | ||
| end | ||
| table.sort(sortedIds) | ||
|
|
||
| -- Iterate direct children of the depends_on array table | ||
| for i = 1, #dependsOn do | ||
| local child = dependsOn[i] | ||
| if child and child.type == 'string' then | ||
| local val = child[1] | ||
| if val and val ~= '*' and not declaredIds[val] then | ||
| callback { | ||
| start = child.start, | ||
| finish = child.finish, | ||
| message = ("devsper: unknown task id '%s' in depends_on — declared ids: %s"):format( | ||
| val, table.concat(sortedIds, ', ') | ||
| ), | ||
| } | ||
| end | ||
| end | ||
| -- Non-string children (variables, function calls) are silently skipped | ||
| end | ||
| end) |
There was a problem hiding this comment.
This section contains two significant issues:
- Correctness Bug: The current implementation iterates over the
depends_ontable and expectsstringnodes as direct children. However, in the Lua Language Server AST, array elements in a table are wrapped intableexpnodes. Consequently,child.type == 'string'will always be false, and the diagnostic will never trigger for unknown task IDs. - Performance Issue: The
sortedIdslist is being rebuilt and sorted inside theeachSourceTypecallback for every function call found in the AST. This is inefficient and should be computed once before starting the second pass.
-- Pass 2: check depends_on entries in each wf.task spec
local sortedIds = {}
for k in pairs(declaredIds) do
sortedIds[#sortedIds + 1] = k
end
table.sort(sortedIds)
local declaredIdsStr = table.concat(sortedIds, ', ')
guide.eachSourceType(state.ast, 'call', function (source)
await.delay()
if not isLocalMethod(source, 'wf', 'task') then return end
local args = source.args
if not args then return end
local spec = args[2]
if not spec or spec.type ~= 'table' then return end
local dependsOn = tableGet(spec, 'depends_on')
if not dependsOn or dependsOn.type ~= 'table' then return end
-- Iterate direct children of the depends_on array table
for i = 1, #dependsOn do
local exp = dependsOn[i]
if exp and exp.type == 'tableexp' then
local child = exp.value
if child and child.type == 'string' then
local val = child[1]
if val and val ~= '*' and not declaredIds[val] then
callback {
start = child.start,
finish = child.finish,
message = ("devsper: unknown task id '%s' in depends_on — declared ids: %s"):format(
val, declaredIdsStr
),
}
end
end
end
-- Non-string children (variables, function calls) are silently skipped
end
end)
| guide.eachSourceType(state.ast, 'call', function (source) | ||
| await.delay() | ||
| if not isLocalMethod(source, 'wf', 'task') then return end | ||
| local args = source.args | ||
| if not args then return end | ||
| local idNode = args[1] | ||
| if not idNode or idNode.type ~= 'string' then return end | ||
| local id = idNode[1] | ||
| if id then | ||
| declaredIds[id] = { start = idNode.start, finish = idNode.finish } | ||
| end | ||
| end) |
There was a problem hiding this comment.
The diagnostic performs two full traversals of all function calls in the AST using guide.eachSourceType. This is inefficient. You can optimize this by performing a single traversal to collect both the declared IDs and the dependency lists that need validation, then performing the validation in a separate loop over the collected data.
| local function isLocalMethod(source, receiverName, methodName) | ||
| if not source or source.type ~= 'call' then return false end | ||
| local callee = source.node | ||
| if not callee or callee.type ~= 'getfield' then return false end | ||
| if not callee.field or callee.field[1] ~= methodName then return false end | ||
| local receiver = callee.node | ||
| if not receiver or receiver.type ~= 'getlocal' then return false end | ||
| return receiver[1] == receiverName | ||
| end |
There was a problem hiding this comment.
The isLocalMethod helper strictly checks for a local variable named wf. This makes the diagnostics brittle, as they will fail to provide feedback if a user renames the variable returned by devsper.workflow() (e.g., local my_wf = devsper.workflow(...)). Consider using the VM's type system to track the object returned by the workflow builder call more robustly.
|
Sorry merged to the wrong repo... |
1 participant
No description provided.