Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request adds comprehensive Language Server Protocol (LSP) support for SuperTOML across three editors: Neovim, VS Code, and Zed. It includes a new Rust-based LSP server, complete editor-specific plugins, updated TOML configuration examples, and core Rust type refactoring to better handle error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor<br/>(Neovim/VSCode/Zed)
participant Plugin as Plugin<br/>Extension
participant LSP as LSP Server
participant Validator as Validator<br/>(parse_toml_config)
User->>Editor: Opens *.super.toml file
Editor->>Plugin: FileType/Activation Event
Plugin->>Plugin: resolve server path
Plugin->>LSP: Start Server Process
User->>Editor: Types/modifies TOML
Editor->>Plugin: didOpen/didChange
Plugin->>LSP: Send document changes
LSP->>Validator: parse_toml_config(text)
Validator->>Validator: Syntax validation
Validator->>Validator: Semantic validation<br/>(dimensions, overrides)
Validator-->>LSP: Vec<TomlError>
LSP->>LSP: Map errors to Diagnostics<br/>(compute_range, format messages)
LSP-->>Plugin: Publish Diagnostics
Plugin-->>Editor: Display error markers
Editor-->>User: Show inline diagnostics
User->>Editor: Trigger completion<br/>at cursor
Editor->>Plugin: completion request
Plugin->>LSP: Send completion request<br/>(text, position)
LSP->>LSP: determine_cursor_context()
LSP->>LSP: Parse TOML defensively
LSP->>LSP: Generate context-specific<br/>completion items
LSP-->>Plugin: CompletionResponse
Plugin-->>Editor: Display completions
Editor-->>User: Show completion menu
User->>Editor: Hover over symbol
Editor->>Plugin: hover request
Plugin->>LSP: Send hover request<br/>(text, position)
LSP->>LSP: Extract word under cursor
LSP->>LSP: Lookup in [dimensions]<br/>or [default-configs]
LSP-->>Plugin: Hover content<br/>(Markdown formatted)
Plugin-->>Editor: Display hover tooltip
Editor-->>User: Show info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
tooling/lsp/neovim-extension/README.md-370-392 (1)
370-392:⚠️ Potential issue | 🟡 MinorAdd language specifier to fenced code block.
The project structure code block should have a language specifier for proper rendering.
📝 Proposed fix
-``` +```text neovim-extension/ ├── lua/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/README.md` around lines 370 - 392, The fenced code block that shows the neovim-extension project tree is missing a language specifier; update the README.md's block that starts with ``` and the tree under "neovim-extension/" to use a plain text specifier (e.g., change ``` to ```text) so the directory tree renders correctly; locate the README block containing the ASCII tree under the neovim-extension header and add the specifier.examples/superposition_toml_example/README.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorInconsistent reference in documentation.
The text mentions "adding them to the
_overrides_table" but the examples above use_context_as the key within[[overrides]]blocks. This should likely read_context_for consistency with the shown examples.📝 Suggested fix
-Contexts define overrides that apply when specific dimension values are present. Multiple dimensions can be combined by adding them to the `_overrides_` table. +Contexts define overrides that apply when specific dimension values are present. Multiple dimensions can be combined by adding them to the `_context_` table.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/superposition_toml_example/README.md` at line 66, The sentence incorrectly refers to the `_overrides_` table; update the documentation to reference the `_context_` key used in the examples instead. Edit the line that currently reads "adding them to the `_overrides_` table" so it instead says "adding them to the `_context_` key" (matching the `[[overrides]]` blocks and the `_context_` symbol shown in the examples).tooling/lsp/vscode-extension/test.super.toml-282-286 (1)
282-286:⚠️ Potential issue | 🟡 MinorIncorrect priority calculation in comment.
The comment states priority includes
2^7(day_of_week at position 7), butday_of_weekis not present in the_context_. The actual priority based on the dimensions in the context should be recalculated.Present dimensions and their positions:
- country (1): 2^1 = 2
- city (2): 2^2 = 4
- zone (3): 2^3 = 8
- vehicle_type (4): 2^4 = 16
- service_tier (5): 2^5 = 32
- hour_of_day (6): 2^6 = 64
- user_tier (8): 2^8 = 256
- city_tier (10): 2^10 = 1024
- experiment_group (13): 2^13 = 8192
Total: 2 + 4 + 8 + 16 + 32 + 64 + 256 + 1024 + 8192 = 9598
📝 Suggested fix
# --- Maximum priority override: all dimensions combined --- # This demonstrates the maximum possible priority combination -# Priority = 2^1 + 2^2 + 2^3 + 2^4 + 2^5 + 2^6 + 2^8 + 2^10 + 2^13 = 8278 +# Priority = 2^1 + 2^2 + 2^3 + 2^4 + 2^5 + 2^6 + 2^8 + 2^10 + 2^13 = 9598🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/vscode-extension/test.super.toml` around lines 282 - 286, The comment above the [[overrides]] block incorrectly lists the priority combination (mentions 2^7/day_of_week and shows 8278); recalc and update it to reflect the actual dimensions present in the _context_ (country, city, zone, vehicle_type, service_tier, hour_of_day, user_tier, city_tier, experiment_group) and replace the incorrect priority value with the correct total 9598 and remove the stray reference to 2^7/day_of_week so the comment matches the _context_ keys.crates/superposition_core/src/toml.rs-342-344 (1)
342-344:⚠️ Potential issue | 🟡 MinorInconsistent error key path format across dimension types.
LocalCohortuses"dimensions.<dim>.schema"(line 342) butRemoteCohort(line 373) andRegular(line 399) still use"<dim>.schema". This inconsistency may confuse users and LSP diagnostics.🔧 Proposed fix for consistency
DimensionType::RemoteCohort(ref cohort_dim) => { // ... validations::validate_schema(&Value::from(&dim_info.schema)) .map_err(|errors| TomlError::ValidationError { - key: format!("{}.schema", dim), + key: format!("dimensions.{}.schema", dim), errors: validations::format_validation_errors(&errors), })?; // ... } DimensionType::Regular {} => { validations::validate_schema(&Value::from(&dim_info.schema)) .map_err(|errors| TomlError::ValidationError { - key: format!("{}.schema", dim), + key: format!("dimensions.{}.schema", dim), errors: validations::format_validation_errors(&errors), })?; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/superposition_core/src/toml.rs` around lines 342 - 344, The error key path is inconsistent: LocalCohort sets key to format!("dimensions.{}.schema", dim) but RemoteCohort and Regular still use format!("{}.schema", dim); update the RemoteCohort and Regular error key constructions in crates::superposition_core::toml.rs (the match arms/handlers that build the error struct using errors: validations::format_validation_errors(&errors)) to use the same "dimensions.<dim>.schema" format as LocalCohort so LSP diagnostics and user-facing keys are consistent.tooling/lsp/vscode-extension/syntaxes/superTOML.tmLanguage.json-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorRemove leading dots from
fileTypesextensions.TextMate grammar
fileTypesmust list extensions without the leading dot. The current format may prevent proper file association.🔧 Suggested fix
- "fileTypes": [".super.toml", ".stoml"], + "fileTypes": ["super.toml", "stoml"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/vscode-extension/syntaxes/superTOML.tmLanguage.json` at line 5, The TextMate grammar's "fileTypes" array currently contains extensions with leading dots (e.g., ".super.toml", ".stoml"); update the "fileTypes" array to list extensions without leading dots (e.g., "super.toml", "stoml") so the editor correctly associates files; locate the "fileTypes" entry in superTOML.tmLanguage.json and remove the leading '.' characters from each extension string.tooling/lsp/vscode-extension/README.md-224-235 (1)
224-235:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced file-structure block.
Line 224 opens a fenced code block without a language, which triggers markdownlint MD040.
💡 Proposed fix
-``` +```text vscode-extension/ ├── package.json # Extension manifest ├── tsconfig.json # TypeScript configuration ├── language-configuration.json # Language settings (brackets, comments) ├── syntaxes/ │ └── superTOML.tmLanguage.json # TextMate grammar for syntax highlighting ├── src/ │ └── extension.ts # Main extension code ├── out/ # Compiled JavaScript (generated) └── README.md # This file</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@tooling/lsp/vscode-extension/README.mdaround lines 224 - 235, The fenced
code block that lists the vscode-extension directory currently has no language
tag (triggering markdownlint MD040); update the README.md fenced block that
starts with "vscode-extension/" to include a language identifier (e.g., add
"text" after the opening) so the block becomestext and resolves the lint
warning while preserving the directory listing content.</details> </blockquote></details> <details> <summary>tooling/lsp/zed-extension/README.md-102-109 (1)</summary><blockquote> `102-109`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the fenced file-structure block.** Line 102 starts a fenced block without a language identifier (markdownlint MD040). <details> <summary>💡 Proposed fix</summary> ```diff -``` +```text zed-extension/ ├── extension.toml # Extension metadata and language configuration ├── Cargo.toml # Rust crate configuration ├── src/ │ └── lib.rs # Extension implementation └── README.md # This file ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@tooling/lsp/zed-extension/README.mdaround lines 102 - 109, The fenced
directory-tree block in README.md (the block showing "zed-extension/ ├──
extension.toml ...") lacks a language tag which triggers markdownlint MD040;
update that fenced code block in README.md to include a language identifier
(e.g., add "text" after the opening) so the block becomestext and
retains the same content for the zed-extension/ tree.</details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua-131-143 (1)</summary><blockquote> `131-143`: _⚠️ Potential issue_ | _🟡 Minor_ **Return value is unreliable due to async deferral.** The function returns `M.client_id` immediately, but the actual restart happens asynchronously in `vim.defer_fn`. The returned value will be `nil` (since `stop()` clears it) or stale, not the new client ID. <details> <summary>🐛 Proposed fix - return nil and document async behavior, or use callback</summary> ```diff --- Restart the LSP server ----@return integer|nil client_id +---@return nil function M.restart() M.stop() -- Small delay to ensure clean shutdown vim.defer_fn(function() M.start() M.attach_to_buffers() end, 100) - return M.client_id + -- Restart is async; client_id will be set after defer_fn completes + return nil end ``` Alternatively, accept a callback parameter to notify when restart completes. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua` around lines 131 - 143, The restart function returns M.client_id immediately even though the new client is created asynchronously in vim.defer_fn, so update M.restart to either (a) document and return nil to reflect asynchronous behavior, or (b) accept a callback or completion handler and invoke it from inside the vim.defer_fn after M.start()/M.attach_to_buffers() with the new M.client_id (or resolve a returned promise-like object); refer to M.restart, M.stop, M.start, M.attach_to_buffers and M.client_id when making the change so the caller receives the correct post-restart client identifier or a clear async contract. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/plugin/init.lua-10-21 (1)</summary><blockquote> `10-21`: _⚠️ Potential issue_ | _🟡 Minor_ **`cpoptions` not restored on early returns.** If the script exits early at lines 6 or 20, the saved `cpoptions` value won't be restored. This could leave Neovim in an unexpected compatibility mode. <details> <summary>🐛 Proposed fix - restore before early returns</summary> ```diff -- Save compatibility options local save_cpo = vim.o.cpoptions vim.o.cpoptions = "" -- Check for Neovim version compatibility if vim.fn.has("nvim-0.9") == 0 then vim.notify( "[supertoml-analyzer] This plugin requires Neovim 0.9 or later.", vim.log.levels.ERROR ) + vim.o.cpoptions = save_cpo return end ``` Or move the version check before modifying `cpoptions`. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/plugin/init.lua` around lines 10 - 21, The code sets save_cpo = vim.o.cpoptions then clears vim.o.cpoptions but doesn't restore it on early exit; ensure vim.o.cpoptions is restored from save_cpo before any early return (e.g., right before the notify+return when vim.fn.has("nvim-0.9") == 0) or alternatively move the version check (vim.fn.has("nvim-0.9")) to run before saving/modifying cpoptions; update all return paths to restore vim.o.cpoptions using save_cpo so the original cpoptions value is always reinstated. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua-148-156 (1)</summary><blockquote> `148-156`: _⚠️ Potential issue_ | _🟡 Minor_ **Incorrect check for existing sign definition.** `vim.fn.sign_getdefined(name)` returns a list of sign definitions, not a single table. The current check `vim.fn.sign_getdefined(name).text == nil` will always be truthy because a list doesn't have a `.text` property. <details> <summary>🐛 Proposed fix</summary> ```diff for _, sign in ipairs(signs) do local name = "DiagnosticSign" .. sign - if vim.fn.sign_getdefined(name).text == nil then + local existing = vim.fn.sign_getdefined(name) + if `#existing` == 0 then vim.fn.sign_define(name, { text = M.get_diagnostic_sign_text(sign), texthl = name, numhl = "", }) end end ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua` around lines 148 - 156, The check for existing sign definitions is wrong because vim.fn.sign_getdefined(name) returns a list; replace the current condition (vim.fn.sign_getdefined(name).text == nil) with a proper emptiness check such as vim.tbl_isempty(vim.fn.sign_getdefined(name)) or testing the first element (vim.fn.sign_getdefined(name)[1] == nil) before calling vim.fn.sign_define; update the block around the local name = "DiagnosticSign" .. sign and calls to vim.fn.sign_getdefined and vim.fn.sign_define (and still use M.get_diagnostic_sign_text(sign) for the text) so signs are only defined when no existing definition is returned. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua-306-312 (1)</summary><blockquote> `306-312`: _⚠️ Potential issue_ | _🟡 Minor_ **Format-on-save condition uses wrong config flag.** The format-on-save logic checks `cfg.diagnostics.enable`, but formatting is conceptually separate from diagnostics. Consider adding a dedicated `format_on_save` configuration option, and making this opt-in since automatic formatting is opinionated. <details> <summary>🔧 Suggested approach</summary> Add to `SuperTOMLConfig`: ```lua format_on_save = { enable = false, -- Opt-in by default } ``` Then check: ```diff - if cfg.diagnostics.enable then + if cfg.format_on_save and cfg.format_on_save.enable then vim.lsp.buf.format({ bufnr = bufnr, timeout_ms = 1000 }) end ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua` around lines 306 - 312, The format-on-save callback currently checks cfg.diagnostics.enable but formatting should be gated by a dedicated option; add a new format_on_save table to the SuperTOMLConfig (e.g., format_on_save = { enable = false }) and update the callback in lsp.lua to read cfg.format_on_save.enable instead of cfg.diagnostics.enable so vim.lsp.buf.format({ bufnr = bufnr, timeout_ms = 1000 }) only runs when the new opt-in flag is true. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/plugin/init.lua-29-34 (1)</summary><blockquote> `29-34`: _⚠️ Potential issue_ | _🟡 Minor_ **Security concern: `vim.fn.eval` can execute arbitrary code.** Using `vim.fn.eval` to parse command arguments could allow code execution if a user passes malicious input. Consider using `vim.json.decode` or a safer parsing method, or simply document that the command accepts Lua table syntax. <details> <summary>🛡️ Safer alternative using loadstring with sandboxing</summary> ```diff if opts.args and opts.args ~= "" then - local ok, parsed = pcall(vim.fn.eval, opts.args) - if ok and type(parsed) == "table" then - config = parsed + -- Only accept simple table literals + local chunk, err = loadstring("return " .. opts.args) + if chunk then + -- Sandbox the execution + setfenv(chunk, {}) + local ok, parsed = pcall(chunk) + if ok and type(parsed) == "table" then + config = parsed + end end end ``` Alternatively, document that this command is intended for trusted interactive use only. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/plugin/init.lua` around lines 29 - 34, The current parsing of command arguments uses vim.fn.eval on opts.args (in the code handling opts.args) which can execute arbitrary code; replace this with a safe parser such as vim.json.decode (or a restricted sandboxed parser) and validate the result is a table before assigning to config, or explicitly document that opts.args must be trusted Lua table syntax; specifically stop calling vim.fn.eval with untrusted input, parse opts.args with vim.json.decode (or another safe decode) and keep the type check for table before setting config. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (12)</summary><blockquote> <details> <summary>tooling/lsp/vscode-extension/.vscodeignore (1)</summary><blockquote> `10-13`: **Minor redundancy in ignore patterns.** Lines 10-11 have overlapping patterns (`**/node_modules/**` already covers `node_modules/**`). Similarly, lines 4-5 (`**/*.ts`, `**/*.map`) already cover the more specific patterns on lines 12-13 (`out/**/*.ts`, `out/**/*.map`). <details> <summary>✨ Suggested cleanup</summary> ```diff **/node_modules/** -node_modules/** out/**/*.ts -out/**/*.map ``` Or alternatively, keep only the broader patterns and remove lines 12-13. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/vscode-extension/.vscodeignore` around lines 10 - 13, Remove redundant ignore patterns: keep the broad patterns (**/node_modules/** and **/*.ts, **/*.map) and delete the overlapping specific entries (node_modules/**, out/**/*.ts, out/**/*.map) so the .vscodeignore only contains the broader rules; adjust so only **/node_modules/** and the generic **/*.ts and **/*.map remain. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/vscode-extension/src/extension.ts (1)</summary><blockquote> `171-187`: **Consider making `checkOnSave` configurable.** The `checkOnSave.enable` is hardcoded to `true` while other options like `diagnostics`, `completions`, and `hover` are configurable via user settings. Consider adding a corresponding configuration option in `package.json` for consistency. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/vscode-extension/src/extension.ts` around lines 171 - 187, getInitializationOptions currently hardcodes checkOnSave.enable to true while other features read user prefs; make checkOnSave configurable by adding a new setting (e.g., "checkOnSave.enable") in package.json and update getInitializationOptions to read workspace.getConfiguration('superTOML').get<boolean>('checkOnSave.enable', true) (or similar) so checkOnSave.enable mirrors diagnostics/completions/hover and uses a sensible default. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/vscode-extension/package.json (1)</summary><blockquote> `25-27`: **Consider removing redundant `activationEvents`.** Since VS Code 1.74+, `activationEvents` can be automatically inferred from `contributes.languages`. The explicit `onLanguage:superTOML` event is redundant when the language is already declared in `contributes.languages`. <details> <summary>♻️ Proposed simplification</summary> ```diff - "activationEvents": [ - "onLanguage:superTOML" - ], "main": "./out/extension.js", ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/vscode-extension/package.json` around lines 25 - 27, Remove the redundant explicit activationEvents entry "onLanguage:superTOML" since VS Code >=1.74 can infer activation from contributes.languages; locate the "activationEvents" array in package.json and delete the "onLanguage:superTOML" item (or remove the entire activationEvents key if it only contains that entry), keeping contributes.languages unchanged. ``` </details> </blockquote></details> <details> <summary>crates/superposition_core/src/toml.rs (1)</summary><blockquote> `545-546`: **Simplify closure to method reference.** The closure `|e| TomlError::TomlSyntaxError(e)` can be simplified. <details> <summary>✨ Suggested simplification</summary> ```diff let detailed_toml_config = toml::from_str::<DetailedConfigToml>(toml_str) - .map_err(|e| TomlError::TomlSyntaxError(e))?; + .map_err(TomlError::TomlSyntaxError)?; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@crates/superposition_core/src/toml.rs` around lines 545 - 546, The closure passed to map_err can be replaced with a direct function pointer; change the call that constructs detailed_toml_config (the toml::from_str::<DetailedConfigToml>(toml_str).map_err(|e| TomlError::TomlSyntaxError(e))?) to use map_err(TomlError::TomlSyntaxError) so the code is simpler and equivalent while still producing the same TomlError variant. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/server/src/main.rs (1)</summary><blockquote> `9-14`: **Consider adding logging initialization for debugging.** LSP servers benefit from logging for debugging client-server communication issues. While optional, adding `tracing` or `env_logger` initialization would help with troubleshooting. <details> <summary>💡 Optional: Add tracing support</summary> ```diff +use tracing_subscriber::EnvFilter; + #[tokio::main] async fn main() { + // Initialize logging (writes to stderr, not interfering with LSP on stdin/stdout) + tracing_subscriber::fmt() + .with_env_filter(EnvFilter::from_default_env()) + .with_writer(std::io::stderr) + .init(); + let stdin = tokio::io::stdin(); let stdout = tokio::io::stdout(); let (service, socket) = LspService::new(backend::Backend::new); Server::new(stdin, stdout, socket).serve(service).await; } ``` This requires adding `tracing` and `tracing-subscriber` to `Cargo.toml`: ```toml tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/server/src/main.rs` around lines 9 - 14, Initialize logging/tracing at the start of main before creating the LSP service: add tracing and tracing-subscriber (or env_logger) to Cargo.toml, then in async fn main call a tracing subscriber initializer (e.g., configure env-filter from RUST_LOG and call tracing_subscriber::fmt::init() or build a subscriber with tracing_subscriber::fmt().with_env_filter(...).init()) prior to invoking LspService::new(backend::Backend::new) and Server::new(...).serve(...).await so all backend::Backend and Server logs are emitted for debugging. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/zed-extension/README.md (1)</summary><blockquote> `3-3`: **Clarify scope in the intro sentence.** Line 3 says “support for TOML files,” which can imply full generic TOML coverage. Consider explicitly saying SuperTOML files to set correct expectations. <details> <summary>💡 Proposed wording</summary> ```diff -A Zed editor extension that provides language server support for TOML files using the `supertoml-analyzer` LSP server. +A Zed editor extension that provides language server support for SuperTOML files using the `supertoml-analyzer` LSP server. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/zed-extension/README.md` at line 3, Update the README intro sentence to clarify scope by specifying that the extension provides language server support for SuperTOML files rather than generic TOML; locate the sentence referencing "TOML files" in the Zed editor extension README (the line mentioning `supertoml-analyzer`) and change it to explicitly say "SuperTOML files" or similar wording that ties it to the SuperTOML format/project to avoid implying full generic TOML coverage. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/ftplugin/superTOML.lua (1)</summary><blockquote> `82-83`: **Use `vim.uv or vim.loop` for Neovim version compatibility.** Line 82 hardcodes `vim.loop`, which is deprecated as of Neovim 0.10. Use `vim.uv or vim.loop` to support both Neovim 0.9.x and 0.10+ without version-specific breakage. <details> <summary>Suggested fix</summary> ```diff -local ok, stats = pcall(vim.loop.fs_stat, vim.api.nvim_buf_get_name(bufnr)) +local uv = vim.uv or vim.loop +local ok, stats = pcall(uv.fs_stat, vim.api.nvim_buf_get_name(bufnr)) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/ftplugin/superTOML.lua` around lines 82 - 83, Replace the hardcoded use of vim.loop with a compatibility alias (e.g., local uv = vim.uv or vim.loop) and call uv.fs_stat(...) instead of vim.loop.fs_stat so the fs_stat call in the block that uses vim.api.nvim_buf_get_name(bufnr), ok, stats and max_filesize works on both Neovim 0.9.x and 0.10+; update the line using vim.loop.fs_stat to use the uv alias (uv.fs_stat) while leaving the surrounding logic (pcall, vim.api.nvim_buf_get_name, bufnr, stats.size > max_filesize) unchanged. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua (2)</summary><blockquote> `173-190`: **Global diagnostic config may override user settings.** `vim.diagnostic.config()` applies globally and will override any existing diagnostic configuration the user may have set. Consider making this opt-in via a configuration option, or document this behavior clearly. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua` around lines 173 - 190, The current M.setup_diagnostics function unconditionally calls vim.diagnostic.config which overwrites global user diagnostics; change it to respect a module configuration option (e.g., a flag like config.enable_diagnostics_setup or similar) or provide an opt-in parameter to M.setup_diagnostics, and only call vim.diagnostic.config when that flag/parameter is true; update the function signature and any initialization paths that call M.setup_diagnostics so consumers can opt in (or document the behavior) and ensure you reference M.setup_diagnostics and the vim.diagnostic.config call when making the conditional guard. ``` </details> --- `77-80`: **Use `vim.bo` instead of deprecated `nvim_buf_get_option`/`nvim_buf_set_option`.** These functions are deprecated since Neovim 0.9. Use the `vim.bo[bufnr]` accessor instead. <details> <summary>♻️ Proposed fix</summary> ```diff callback = function(args) - local current_ft = vim.api.nvim_buf_get_option(args.buf, "filetype") + local current_ft = vim.bo[args.buf].filetype if current_ft == "" or current_ft == "toml" then - vim.api.nvim_buf_set_option(args.buf, "filetype", "superTOML") + vim.bo[args.buf].filetype = "superTOML" end end, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua` around lines 77 - 80, Replace deprecated buffer option APIs by reading and writing the filetype through the buffer-local table accessor: read the current filetype from vim.bo[args.buf].filetype (instead of vim.api.nvim_buf_get_option) into current_ft and set it via vim.bo[args.buf].filetype = "superTOML" (instead of vim.api.nvim_buf_set_option). Ensure you still check for empty string or "toml" before assigning, using the existing variables (current_ft and args.buf) to locate the change. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua (1)</summary><blockquote> `180-182`: **Consider making `checkOnSave` configurable.** The `checkOnSave` option is hardcoded to `enable = true`. If this is intentional, it's fine, but consider exposing it through the configuration for consistency with other options. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua` around lines 180 - 182, The checkOnSave setting is hardcoded (checkOnSave = { enable = true })—make it configurable by reading a user-provided option (e.g., options.checkOnSave or config.checkOnSave) with a sensible default of true; update the configuration table where checkOnSave is defined to use that option, and ensure any code that references checkOnSave (the checkOnSave table or its enable flag) uses the new configurable value so behavior remains backward-compatible. ``` </details> </blockquote></details> <details> <summary>tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua (2)</summary><blockquote> `341-352`: **Use `vim.bo` instead of deprecated `nvim_buf_set_option`.** Same as noted earlier, `nvim_buf_set_option` is deprecated since Neovim 0.9. <details> <summary>♻️ Proposed fix</summary> ```diff callback = function(args) if M.is_super_toml_buffer(args.buf) then - vim.api.nvim_buf_set_option(args.buf, "filetype", M.get_filetype()) + vim.bo[args.buf].filetype = M.get_filetype() end end, }) -- Auto-attach to newly created SuperTOML files vim.api.nvim_create_autocmd("BufNewFile", { group = M.augroup, pattern = M.file_patterns, desc = "Set filetype for new SuperTOML files", callback = function(args) - vim.api.nvim_buf_set_option(args.buf, "filetype", M.get_filetype()) + vim.bo[args.buf].filetype = M.get_filetype() end, }) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua` around lines 341 - 352, Replace deprecated vim.api.nvim_buf_set_option usage with the buffer-local API (vim.bo) when setting the filetype: in the BufNewFile autocmd callback (and the earlier autocmd callback above) stop calling vim.api.nvim_buf_set_option(args.buf, "filetype", M.get_filetype()) and instead assign the option via the buffer-local table using args.buf (e.g., set vim.bo[<buffer>].filetype = M.get_filetype()); update both places where args.buf and M.get_filetype() are used so the filetype is set using vim.bo rather than the deprecated nvim_buf_set_option. ``` </details> --- `265-294`: **Consider making keymaps configurable or opt-in.** The plugin sets buffer-local keymaps unconditionally, which may override user customizations. Consider adding a configuration option like `keymaps = { enable = true }` or allowing users to provide their own keymap table. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua` around lines 265 - 294, The current M.setup_buffer_keymaps(bufnr) unconditionally sets buffer-local keymaps and can override user mappings; change it to respect a configuration table (e.g., a module-level config.keymaps.enable boolean and optional config.keymaps.bindings table) so keymaps are only applied when enabled and allow users to supply/merge their own mapping table; update M.setup_buffer_keymaps to accept or read the config, check config.keymaps.enable before calling vim.keymap.set, and if config.keymaps.bindings is present iterate that table (falling back to the default mappings) to set each mapping so users can opt-in or override defaults. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua:
- Line 1: Remove the stray file path string on the first line of the module (the
literal
"superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua")
so the Lua file parses correctly; open the module (config.lua) and delete that
top-line path entry, ensuring the file begins with the intended Lua code (e.g.,
local config = ... or return {...}) and that any module-level symbols like the
config table or return statement remain intact.In
@tooling/lsp/neovim-extension/queries/supertoml/highlights.scm:
- Around line 8-27: The Tree-sitter queries for table headers incorrectly
require both (bare_key) and (dotted_key) as children, so patterns like the
(table) and (table_array_element) captures never match; update each
table/table_array_element pattern (the ones capturing@namespaceand
@punctuation.bracket) to use alternation for the key child — e.g., replace
sequences requiring both (bare_key) and (dotted_key) with a single bracketed
alternative that includes (bare_key) (dotted_key) and (quoted_key) as
appropriate — so the@namespacecapture matches any single key child and the
bracket captures remain on the surrounding "["/"]" or "[["/"]]".In
@tooling/lsp/neovim-extension/queries/supertoml/injections.scm:
- Around line 5-27: The string capture
@injection.contentincludes surrounding
quotes so anchoring patterns like ^ match the delimiter instead of the payload;
for each injection rule that uses ((string)@injection.content...) (the ones
setting injection.language to "bash", "regex", "json", and the two "lua" rules),
add an offset directive to unwrap the quotes by inserting#offset!
@injection.content0 1 0 -1 inside the rule so the regex matches the inner
content rather than the delimiters.In
@tooling/lsp/server/src/backend.rs:
- Around line 31-56: The initialize handler currently does not negotiate
position encoding, causing UTF-16/byte-offset mismatches; update async fn
initialize(&self, params: InitializeParams) to inspect
params.capabilities.general.position_encodings (or the equivalent capability
path) and choose an encoding: prefer PositionEncodingKind::UTF8 if the client
advertises it, otherwise fall back to PositionEncodingKind::UTF16; store the
chosen encoding in the server state and set ServerCapabilities.position_encoding
to that choice so clients know what the server uses; ensure callers that rely on
byte offsets (e.g., byte_offset_to_position, hover.rs, utils.rs, diagnostics.rs)
either read the stored encoding and convert between byte offsets and LSP
positions accordingly or are updated later to expect the negotiated encoding.In
@tooling/lsp/server/src/diagnostics.rs:
- Around line 143-145: The code slices intermediate segments into sub_nesting
but only uses sub_nesting[0], so nested TOML paths like
dimensions.peak_hours.schema.type stop one level early; update the resolver that
references sub_nesting to iteratively traverse each segment in sub_nesting
(e.g., starting from the root table/value, loop over each segment name, descend
into its table/inline table or array element as appropriate, and update the
current context) rather than indexing [0], and fall back gracefully when a key
is missing; apply the same change to the other resolver occurrences noted (the
blocks around the other sub_nesting uses).In
@tooling/lsp/server/src/hover.rs:
- Around line 20-23: The code currently does a hard early return using
toml::from_str(text).ok()? which disables hover on any TOML syntax error; change
this to a best-effort approach by capturing the parse result as an Option (e.g.
let raw: Optiontoml::Table = toml::from_str(text).ok();) instead of using the
? operator, and update subsequent uses of raw in hover.rs to handle None
gracefully (mirror the pattern used in completions.rs) so hover can still
operate using the raw buffer or partial data when parsing fails.In
@tooling/lsp/vscode-extension/src/extension.ts:
- Around line 132-163: The workspace/extension path discovery builds
possiblePaths without Windows executables, so on Windows the searches miss the
real binary; update the code that populates possiblePaths (the blocks
referencing workspace.workspaceFolders and extensionRoot) to also include the
'.exe' variant when process.platform === 'win32' (or push both with and without
'.exe'), ensuring the same Windows-handling used in the PATH fallback is applied
to the checks that use possiblePaths and resolvedPath.In
@tooling/lsp/zed-extension/languages/superTOML/redactions.scm:
- Line 1: The current S-expression rule (pair (bare_key) "=" (_)
@redact)
redacts all TOML values; restrict it by adding a#match? conditional on the
bare_key so only known sensitive keys are redacted (e.g.,
"password","secret","api_key","token" or the project's canonical sensitive-key
list). Locate the pattern that uses bare_key and@redactand wrap the value
capture with a conditional check (using#match? against the sensitive-key regex
or list) so non-sensitive pairs are not assigned@redactwhile preserving
existing token names (bare_key,@redact).In
@tooling/lsp/zed-extension/src/lib.rs:
- Around line 20-34: Replace the hard-coded machine-specific path by resolving
the analyzer binary at runtime: remove the literal "/Users/..." string in the
Command returned and instead look up the bundled/packaged binary (via
environment/config, PATH lookup, or extension resource loading) and populate
self.cached_binary_path when found; if resolution fails return an Err (or a
clear failure Command) rather than trying to spawn a non-existent path. Update
the code that constructs Command (the block that currently returns Command {
command: "...".to_string(), ... }) to use the resolved path and ensure
cached_binary_path is set so the cache branch is exercised.
Minor comments:
In@crates/superposition_core/src/toml.rs:
- Around line 342-344: The error key path is inconsistent: LocalCohort sets key
to format!("dimensions.{}.schema", dim) but RemoteCohort and Regular still use
format!("{}.schema", dim); update the RemoteCohort and Regular error key
constructions in crates::superposition_core::toml.rs (the match arms/handlers
that build the error struct using errors:
validations::format_validation_errors(&errors)) to use the same
"dimensions..schema" format as LocalCohort so LSP diagnostics and
user-facing keys are consistent.In
@examples/superposition_toml_example/README.md:
- Line 66: The sentence incorrectly refers to the
_overrides_table; update
the documentation to reference the_context_key used in the examples instead.
Edit the line that currently reads "adding them to the_overrides_table" so
it instead says "adding them to the_context_key" (matching the
[[overrides]]blocks and the_context_symbol shown in the examples).In
@tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua:
- Around line 148-156: The check for existing sign definitions is wrong because
vim.fn.sign_getdefined(name) returns a list; replace the current condition
(vim.fn.sign_getdefined(name).text == nil) with a proper emptiness check such as
vim.tbl_isempty(vim.fn.sign_getdefined(name)) or testing the first element
(vim.fn.sign_getdefined(name)[1] == nil) before calling vim.fn.sign_define;
update the block around the local name = "DiagnosticSign" .. sign and calls to
vim.fn.sign_getdefined and vim.fn.sign_define (and still use
M.get_diagnostic_sign_text(sign) for the text) so signs are only defined when no
existing definition is returned.In
@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua:
- Around line 131-143: The restart function returns M.client_id immediately even
though the new client is created asynchronously in vim.defer_fn, so update
M.restart to either (a) document and return nil to reflect asynchronous
behavior, or (b) accept a callback or completion handler and invoke it from
inside the vim.defer_fn after M.start()/M.attach_to_buffers() with the new
M.client_id (or resolve a returned promise-like object); refer to M.restart,
M.stop, M.start, M.attach_to_buffers and M.client_id when making the change so
the caller receives the correct post-restart client identifier or a clear async
contract.- Around line 306-312: The format-on-save callback currently checks
cfg.diagnostics.enable but formatting should be gated by a dedicated option; add
a new format_on_save table to the SuperTOMLConfig (e.g., format_on_save = {
enable = false }) and update the callback in lsp.lua to read
cfg.format_on_save.enable instead of cfg.diagnostics.enable so
vim.lsp.buf.format({ bufnr = bufnr, timeout_ms = 1000 }) only runs when the new
opt-in flag is true.In
@tooling/lsp/neovim-extension/plugin/init.lua:
- Around line 10-21: The code sets save_cpo = vim.o.cpoptions then clears
vim.o.cpoptions but doesn't restore it on early exit; ensure vim.o.cpoptions is
restored from save_cpo before any early return (e.g., right before the
notify+return when vim.fn.has("nvim-0.9") == 0) or alternatively move the
version check (vim.fn.has("nvim-0.9")) to run before saving/modifying cpoptions;
update all return paths to restore vim.o.cpoptions using save_cpo so the
original cpoptions value is always reinstated.- Around line 29-34: The current parsing of command arguments uses vim.fn.eval
on opts.args (in the code handling opts.args) which can execute arbitrary code;
replace this with a safe parser such as vim.json.decode (or a restricted
sandboxed parser) and validate the result is a table before assigning to config,
or explicitly document that opts.args must be trusted Lua table syntax;
specifically stop calling vim.fn.eval with untrusted input, parse opts.args with
vim.json.decode (or another safe decode) and keep the type check for table
before setting config.In
@tooling/lsp/neovim-extension/README.md:
- Around line 370-392: The fenced code block that shows the neovim-extension
project tree is missing a language specifier; update the README.md's block that
starts withand the tree under "neovim-extension/" to use a plain text specifier (e.g., changeto ```text) so the directory tree renders correctly;
locate the README block containing the ASCII tree under the neovim-extension
header and add the specifier.In
@tooling/lsp/vscode-extension/README.md:
- Around line 224-235: The fenced code block that lists the vscode-extension
directory currently has no language tag (triggering markdownlint MD040); update
the README.md fenced block that starts with "vscode-extension/" to include a
language identifier (e.g., add "text" after the opening) so the block becomestext and resolves the lint warning while preserving the directory
listing content.In
@tooling/lsp/vscode-extension/syntaxes/superTOML.tmLanguage.json:
- Line 5: The TextMate grammar's "fileTypes" array currently contains extensions
with leading dots (e.g., ".super.toml", ".stoml"); update the "fileTypes" array
to list extensions without leading dots (e.g., "super.toml", "stoml") so the
editor correctly associates files; locate the "fileTypes" entry in
superTOML.tmLanguage.json and remove the leading '.' characters from each
extension string.In
@tooling/lsp/vscode-extension/test.super.toml:
- Around line 282-286: The comment above the [[overrides]] block incorrectly
lists the priority combination (mentions 2^7/day_of_week and shows 8278); recalc
and update it to reflect the actual dimensions present in the context
(country, city, zone, vehicle_type, service_tier, hour_of_day, user_tier,
city_tier, experiment_group) and replace the incorrect priority value with the
correct total 9598 and remove the stray reference to 2^7/day_of_week so the
comment matches the context keys.In
@tooling/lsp/zed-extension/README.md:
- Around line 102-109: The fenced directory-tree block in README.md (the block
showing "zed-extension/ ├── extension.toml ...") lacks a language tag which
triggers markdownlint MD040; update that fenced code block in README.md to
include a language identifier (e.g., add "text" after the opening) so the block becomestext and retains the same content for the zed-extension/ tree.
Nitpick comments:
In@crates/superposition_core/src/toml.rs:
- Around line 545-546: The closure passed to map_err can be replaced with a
direct function pointer; change the call that constructs detailed_toml_config
(the toml::from_str::(toml_str).map_err(|e|
TomlError::TomlSyntaxError(e))?) to use map_err(TomlError::TomlSyntaxError) so
the code is simpler and equivalent while still producing the same TomlError
variant.In
@tooling/lsp/neovim-extension/ftplugin/superTOML.lua:
- Around line 82-83: Replace the hardcoded use of vim.loop with a compatibility
alias (e.g., local uv = vim.uv or vim.loop) and call uv.fs_stat(...) instead of
vim.loop.fs_stat so the fs_stat call in the block that uses
vim.api.nvim_buf_get_name(bufnr), ok, stats and max_filesize works on both
Neovim 0.9.x and 0.10+; update the line using vim.loop.fs_stat to use the uv
alias (uv.fs_stat) while leaving the surrounding logic (pcall,
vim.api.nvim_buf_get_name, bufnr, stats.size > max_filesize) unchanged.In
@tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua:
- Around line 180-182: The checkOnSave setting is hardcoded (checkOnSave = {
enable = true })—make it configurable by reading a user-provided option (e.g.,
options.checkOnSave or config.checkOnSave) with a sensible default of true;
update the configuration table where checkOnSave is defined to use that option,
and ensure any code that references checkOnSave (the checkOnSave table or its
enable flag) uses the new configurable value so behavior remains
backward-compatible.In
@tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua:
- Around line 173-190: The current M.setup_diagnostics function unconditionally
calls vim.diagnostic.config which overwrites global user diagnostics; change it
to respect a module configuration option (e.g., a flag like
config.enable_diagnostics_setup or similar) or provide an opt-in parameter to
M.setup_diagnostics, and only call vim.diagnostic.config when that
flag/parameter is true; update the function signature and any initialization
paths that call M.setup_diagnostics so consumers can opt in (or document the
behavior) and ensure you reference M.setup_diagnostics and the
vim.diagnostic.config call when making the conditional guard.- Around line 77-80: Replace deprecated buffer option APIs by reading and
writing the filetype through the buffer-local table accessor: read the current
filetype from vim.bo[args.buf].filetype (instead of vim.api.nvim_buf_get_option)
into current_ft and set it via vim.bo[args.buf].filetype = "superTOML" (instead
of vim.api.nvim_buf_set_option). Ensure you still check for empty string or
"toml" before assigning, using the existing variables (current_ft and args.buf)
to locate the change.In
@tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua:
- Around line 341-352: Replace deprecated vim.api.nvim_buf_set_option usage with
the buffer-local API (vim.bo) when setting the filetype: in the BufNewFile
autocmd callback (and the earlier autocmd callback above) stop calling
vim.api.nvim_buf_set_option(args.buf, "filetype", M.get_filetype()) and instead
assign the option via the buffer-local table using args.buf (e.g., set
vim.bo[].filetype = M.get_filetype()); update both places where args.buf
and M.get_filetype() are used so the filetype is set using vim.bo rather than
the deprecated nvim_buf_set_option.- Around line 265-294: The current M.setup_buffer_keymaps(bufnr) unconditionally
sets buffer-local keymaps and can override user mappings; change it to respect a
configuration table (e.g., a module-level config.keymaps.enable boolean and
optional config.keymaps.bindings table) so keymaps are only applied when enabled
and allow users to supply/merge their own mapping table; update
M.setup_buffer_keymaps to accept or read the config, check config.keymaps.enable
before calling vim.keymap.set, and if config.keymaps.bindings is present iterate
that table (falling back to the default mappings) to set each mapping so users
can opt-in or override defaults.In
@tooling/lsp/server/src/main.rs:
- Around line 9-14: Initialize logging/tracing at the start of main before
creating the LSP service: add tracing and tracing-subscriber (or env_logger) to
Cargo.toml, then in async fn main call a tracing subscriber initializer (e.g.,
configure env-filter from RUST_LOG and call tracing_subscriber::fmt::init() or
build a subscriber with tracing_subscriber::fmt().with_env_filter(...).init())
prior to invoking LspService::new(backend::Backend::new) and
Server::new(...).serve(...).await so all backend::Backend and Server logs are
emitted for debugging.In
@tooling/lsp/vscode-extension/.vscodeignore:
- Around line 10-13: Remove redundant ignore patterns: keep the broad patterns
(/node_modules/ and /.ts, **/.map) and delete the overlapping specific
entries (node_modules/, out//*.ts, out//.map) so the .vscodeignore only
contains the broader rules; adjust so only /node_modules/ and the generic
**/.ts and **/*.map remain.In
@tooling/lsp/vscode-extension/package.json:
- Around line 25-27: Remove the redundant explicit activationEvents entry
"onLanguage:superTOML" since VS Code >=1.74 can infer activation from
contributes.languages; locate the "activationEvents" array in package.json and
delete the "onLanguage:superTOML" item (or remove the entire activationEvents
key if it only contains that entry), keeping contributes.languages unchanged.In
@tooling/lsp/vscode-extension/src/extension.ts:
- Around line 171-187: getInitializationOptions currently hardcodes
checkOnSave.enable to true while other features read user prefs; make
checkOnSave configurable by adding a new setting (e.g., "checkOnSave.enable") in
package.json and update getInitializationOptions to read
workspace.getConfiguration('superTOML').get('checkOnSave.enable', true)
(or similar) so checkOnSave.enable mirrors diagnostics/completions/hover and
uses a sensible default.In
@tooling/lsp/zed-extension/README.md:
- Line 3: Update the README intro sentence to clarify scope by specifying that
the extension provides language server support for SuperTOML files rather than
generic TOML; locate the sentence referencing "TOML files" in the Zed editor
extension README (the line mentioningsupertoml-analyzer) and change it to
explicitly say "SuperTOML files" or similar wording that ties it to the
SuperTOML format/project to avoid implying full generic TOML coverage.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `e692a85c-d203-4f31-86c2-1a55a3de036f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 95468ee596eca5161e921a9b54422c416a6696c2 and af4545048ba402f91a11b3ead34f3ee399c6604a. </details> <details> <summary>⛔ Files ignored due to path filters (4)</summary> * `Cargo.lock` is excluded by `!**/*.lock` * `crates/cac_toml/Cargo.lock` is excluded by `!**/*.lock` * `tooling/lsp/vscode-extension/icons/toml-icon.svg` is excluded by `!**/*.svg` * `tooling/lsp/vscode-extension/package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (47)</summary> * `Cargo.toml` * `clients/java/bindings/README_TOML_TESTS.md` * `clients/javascript/bindings/README_TOML_TESTS.md` * `crates/superposition_core/src/toml.rs` * `examples/superposition_toml_example/README.md` * `tooling/lsp/neovim-extension/.gitignore` * `tooling/lsp/neovim-extension/README.md` * `tooling/lsp/neovim-extension/ftdetect/supertoml.lua` * `tooling/lsp/neovim-extension/ftplugin/superTOML.lua` * `tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua` * `tooling/lsp/neovim-extension/lua/supertoml-analyzer/init.lua` * `tooling/lsp/neovim-extension/lua/supertoml-analyzer/lsp.lua` * `tooling/lsp/neovim-extension/plugin/init.lua` * `tooling/lsp/neovim-extension/queries/supertoml/highlights.scm` * `tooling/lsp/neovim-extension/queries/supertoml/injections.scm` * `tooling/lsp/neovim-extension/queries/supertoml/textobjects.scm` * `tooling/lsp/neovim-extension/syntax/superTOML.vim` * `tooling/lsp/server/Cargo.toml` * `tooling/lsp/server/src/backend.rs` * `tooling/lsp/server/src/completions.rs` * `tooling/lsp/server/src/diagnostics.rs` * `tooling/lsp/server/src/hover.rs` * `tooling/lsp/server/src/main.rs` * `tooling/lsp/server/src/utils.rs` * `tooling/lsp/vscode-extension/.gitignore` * `tooling/lsp/vscode-extension/.vscodeignore` * `tooling/lsp/vscode-extension/README.md` * `tooling/lsp/vscode-extension/language-configuration.json` * `tooling/lsp/vscode-extension/package.json` * `tooling/lsp/vscode-extension/src/extension.ts` * `tooling/lsp/vscode-extension/syntaxes/superTOML.tmLanguage.json` * `tooling/lsp/vscode-extension/test.super.toml` * `tooling/lsp/vscode-extension/tsconfig.json` * `tooling/lsp/zed-extension/.gitignore` * `tooling/lsp/zed-extension/Cargo.toml` * `tooling/lsp/zed-extension/README.md` * `tooling/lsp/zed-extension/extension.toml` * `tooling/lsp/zed-extension/languages/superTOML/brackets.scm` * `tooling/lsp/zed-extension/languages/superTOML/config.toml` * `tooling/lsp/zed-extension/languages/superTOML/highlights.scm` * `tooling/lsp/zed-extension/languages/superTOML/indents.scm` * `tooling/lsp/zed-extension/languages/superTOML/injections.scm` * `tooling/lsp/zed-extension/languages/superTOML/outline.scm` * `tooling/lsp/zed-extension/languages/superTOML/overrides.scm` * `tooling/lsp/zed-extension/languages/superTOML/redactions.scm` * `tooling/lsp/zed-extension/languages/superTOML/textobjects.scm` * `tooling/lsp/zed-extension/src/lib.rs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @@ -0,0 +1,212 @@ | |||
| superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua | |||
Contributor
There was a problem hiding this comment.
Critical: File path accidentally included as code.
Line 1 contains the file path superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua which is not valid Lua syntax. This will cause a parse error when the module is loaded, breaking the entire plugin.
🐛 Proposed fix - remove the erroneous line
-superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua
---@class SuperTOMLConfig
---@field server SuperTOMLServerConfig📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua | |
| ---@class SuperTOMLConfig | |
| ---@field server SuperTOMLServerConfig |
🧰 Tools
🪛 Luacheck (1.2.0)
[error] 1-1: expected '=' near 'tooling'
(E011)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua` at line 1,
Remove the stray file path string on the first line of the module (the literal
"superposition/tooling/lsp/neovim-extension/lua/supertoml-analyzer/config.lua")
so the Lua file parses correctly; open the module (config.lua) and delete that
top-line path entry, ensuring the file begins with the intended Lua code (e.g.,
local config = ... or return {...}) and that any module-level symbols like the
config table or return statement remain intact.
Comment on lines
+8
to
+27
| (table | ||
| (bare_key) @namespace | ||
| (dotted_key) @namespace) | ||
|
|
||
| (table | ||
| "[" | ||
| (bare_key) @namespace | ||
| (dotted_key) @namespace | ||
| "]") @punctuation.bracket | ||
|
|
||
| ; Array of tables | ||
| (table_array_element | ||
| "[[" @punctuation.bracket | ||
| (bare_key) @namespace | ||
| (dotted_key) @namespace | ||
| "]]" @punctuation.bracket) | ||
|
|
||
| (table_array_element | ||
| (bare_key) @namespace | ||
| (dotted_key) @namespace) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine its content
find . -path "*/tooling/lsp/neovim-extension/queries/supertoml/highlights.scm" -type fRepository: juspay/superposition
Length of output: 127
🏁 Script executed:
# List files in the supertoml queries directory to understand the structure
find . -path "*/queries/supertoml/*" -type fRepository: juspay/superposition
Length of output: 256
🏁 Script executed:
# Look for any TOML grammar files or tree-sitter TOML references
find . -name "*toml*" -type f | head -20Repository: juspay/superposition
Length of output: 789
🏁 Script executed:
# Read the highlights.scm file to examine the patterns
cat -n ./tooling/lsp/neovim-extension/queries/supertoml/highlights.scmRepository: juspay/superposition
Length of output: 2204
🏁 Script executed:
# Check for tree-sitter TOML grammar documentation or references in the repo
find . -name "*.rs" -type f | xargs grep -l "tree-sitter\|toml.*grammar" | head -10Repository: juspay/superposition
Length of output: 46
🌐 Web query:
tree-sitter TOML grammar table node structure
💡 Result:
In the tree-sitter TOML grammar, a “table” header is represented by one of two named node types:
tablefor[table]table_array_elementfor[[table]](array-of-tables element)
Concrete structure (what children you’ll see)
From the grammar rules:
tableparses as:'['+ key +']'+ line-ending + 0+ pairs/newlines. [1]table_array_elementparses as:'[['+ key +']]'+ line-ending + 0+ pairs/newlines. [1]
The key in both cases is either:
dotted_key, or- a single
_key(which becomesbare_keyorquoted_key). [1]
What node-types.json says about the AST shape
node-types.json (the authoritative “node structure” file tree-sitter generates) shows:
tablehas required children drawn from:bare_key | quoted_key | dotted_key | pair(no field names; it’s positional/sequence-based). [2]table_array_elementhas the same child-type set:bare_key | quoted_key | dotted_key | pair. [2]
Because there are no named fields on table / table_array_element, you typically interpret them as:
- first named child = the table key (
bare_key/quoted_key/dotted_key) - remaining named children of type
pair= key/value pairs inside that table (until the next table header). [1][2]
(Background: tree-sitter exposes this via node-types.json’s children/fields metadata.) [3]
Sources: [1] [2] [3]
🌐 Web query:
tree-sitter query syntax matching multiple children
💡 Result:
In Tree-sitter queries, “multiple children” are matched by writing multiple child patterns inside the parent pattern, and they match the parent’s named children in source order (optionally constrained with . anchors). (tree-sitter.github.io)
1) Match a parent that contains two specific children (in order)
(parent_node
(child_a) `@a`
(child_b) `@b`)2) Allow other children in between
Use a wildcard child (often quantified) between them:
(parent_node
(child_a) `@a`
(_)* ; any number of named children in between
(child_b) `@b`)Quantifiers * + ? work on nodes and on grouped sequences. (tree-sitter.github.io)
3) Require the children to be immediate siblings
Put the anchor . between them:
(parent_node
(child_a) `@a`
.
(child_b) `@b`)An anchor between child patterns enforces “immediately adjacent named siblings”. (tree-sitter.github.io)
4) If the two children can appear in either order
Queries are order-sensitive, so you typically write two patterns (or an alternation) to cover both:
[
(parent_node (child_a) `@a` (_)* (child_b) `@b`)
(parent_node (child_b) `@b` (_)* (child_a) `@a`)
]Alternations are written with [...]. (tree-sitter.github.io)
Citations:
- 1: https://tree-sitter.github.io/tree-sitter/using-parsers/queries/2-operators.html
- 2: https://tree-sitter.github.io/tree-sitter/using-parsers/queries/2-operators.html
- 3: https://tree-sitter.github.io/tree-sitter/using-parsers/queries/2-operators.html
- 4: https://tree-sitter.github.io/tree-sitter/using-parsers/queries/2-operators.html
Table header highlight queries never match as written.
In tree-sitter TOML, a table or table_array_element node contains exactly one key child (either bare_key, quoted_key, or dotted_key). The patterns at lines 8–10, 12–16, 19–23, and 25–27 require both bare_key and dotted_key as direct children, making them structurally impossible to match. As a result, namespace and bracket highlighting for all table and array-of-tables headers is completely ineffective.
Use alternations (e.g., [(bare_key) (dotted_key)]) to match either key type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/neovim-extension/queries/supertoml/highlights.scm` around lines 8
- 27, The Tree-sitter queries for table headers incorrectly require both
(bare_key) and (dotted_key) as children, so patterns like the (table) and
(table_array_element) captures never match; update each
table/table_array_element pattern (the ones capturing `@namespace` and
`@punctuation.bracket`) to use alternation for the key child — e.g., replace
sequences requiring both (bare_key) and (dotted_key) with a single bracketed
alternative that includes (bare_key) (dotted_key) and (quoted_key) as
appropriate — so the `@namespace` capture matches any single key child and the
bracket captures remain on the surrounding "["/"]" or "[["/"]]".
Comment on lines
+5
to
+27
| ((string) @injection.content | ||
| (#match? @injection.content "^[\\s]*#!") | ||
| (#set! injection.language "bash")) | ||
|
|
||
| ;; Inject regex for pattern values | ||
| ((string) @injection.content | ||
| (#match? @injection.content "^/.*/$") | ||
| (#set! injection.language "regex")) | ||
|
|
||
| ;; Inject JSON for values that look like JSON | ||
| ((string) @injection.content | ||
| (#match? @injection.content "^\\s*[\\[{].*[\\]}]\\s*$") | ||
| (#set! injection.language "json")) | ||
|
|
||
| ;; Inject Lua for Neovim-style config values | ||
| ((string) @injection.content | ||
| (#match? @injection.content "^\\s*function\\s*\\(") | ||
| (#set! injection.language "lua")) | ||
|
|
||
| ;; Inject Lua for vimscript-style values | ||
| ((string) @injection.content | ||
| (#match? @injection.content "^\\s*vim\\.") | ||
| (#set! injection.language "lua")) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Neovim Tree-sitter injections with TOML grammar, does (string) @injection.content`` include quote delimiters, and is #offset! the recommended way to match unquoted content?
💡 Result:
Yes. In the current tree-sitter-toml grammar, a TOML string node is defined as a sequence that includes the opening and closing quote tokens (e.g. " … " or ' … ', and likewise """ / '''), so capturing (string) @injection.content`` will include the delimiters in the captured text/range. [1]
And yes: in Neovim, #offset! is the standard/recommended way to make an injection operate on a sub-range of a captured node (it rewrites metadata[capture].range and is explicitly called out as useful for injections). [2] For TOML strings, that typically means offsetting by 1/-1 for single-quote/single-double-quote strings, or 3/-3 for triple-quoted multiline strings. [1][2]
Apply #offset! to unwrap string delimiters in injection rules.
In tree-sitter-toml, (string) @injection.content`` captures the node including quote delimiters. This causes patterns anchored with ^ to match against the opening quote rather than the payload, preventing these injection rules from firing. Add `#offset! `@injection.content` 0 1 0 -1` to each rule to trim delimiters:
Proposed fix
((string) `@injection.content`
+ (`#offset`! `@injection.content` 0 1 0 -1)
(`#match`? `@injection.content` "^[\\s]*#!")
(`#set`! injection.language "bash"))
;; Inject regex for pattern values
((string) `@injection.content`
+ (`#offset`! `@injection.content` 0 1 0 -1)
(`#match`? `@injection.content` "^/.*/$")
(`#set`! injection.language "regex"))
;; Inject JSON for values that look like JSON
((string) `@injection.content`
+ (`#offset`! `@injection.content` 0 1 0 -1)
(`#match`? `@injection.content` "^\\s*[\\[{].*[\\]}]\\s*$")
(`#set`! injection.language "json"))
;; Inject Lua for Neovim-style config values
((string) `@injection.content`
+ (`#offset`! `@injection.content` 0 1 0 -1)
(`#match`? `@injection.content` "^\\s*function\\s*\\(")
(`#set`! injection.language "lua"))
;; Inject Lua for vimscript-style values
((string) `@injection.content`
+ (`#offset`! `@injection.content` 0 1 0 -1)
(`#match`? `@injection.content` "^\\s*vim\\.")
(`#set`! injection.language "lua"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/neovim-extension/queries/supertoml/injections.scm` around lines 5
- 27, The string capture `@injection.content` includes surrounding quotes so
anchoring patterns like ^ match the delimiter instead of the payload; for each
injection rule that uses ((string) `@injection.content` ...) (the ones setting
injection.language to "bash", "regex", "json", and the two "lua" rules), add an
offset directive to unwrap the quotes by inserting `#offset`! `@injection.content` 0
1 0 -1 inside the rule so the regex matches the inner content rather than the
delimiters.
Comment on lines
+31
to
+56
| async fn initialize(&self, _: InitializeParams) -> Result<InitializeResult> { | ||
| self.client | ||
| .log_message(MessageType::INFO, "Initializing superTOML Analyzer...") | ||
| .await; | ||
| Ok(InitializeResult { | ||
| capabilities: ServerCapabilities { | ||
| text_document_sync: Some(TextDocumentSyncCapability::Kind( | ||
| TextDocumentSyncKind::FULL, | ||
| )), | ||
| completion_provider: Some(CompletionOptions { | ||
| trigger_characters: Some(vec![ | ||
| "=".to_string(), | ||
| " ".to_string(), | ||
| "{".to_string(), | ||
| ",".to_string(), | ||
| ]), | ||
| ..Default::default() | ||
| }), | ||
| hover_provider: Some(HoverProviderCapability::Simple(true)), | ||
| ..Default::default() | ||
| }, | ||
| server_info: Some(ServerInfo { | ||
| name: "supertoml-analyzer".to_string(), | ||
| version: Some(env!("CARGO_PKG_VERSION").to_string()), | ||
| }), | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: locate byte-based position math and any negotiated position encoding.
# Expectation: direct byte-based handling is present, and no `position_encoding`
# capability is configured in the server.
rg -n 'character as usize|byte_offset_to_position|position_encoding|PositionEncoding' tooling/lsp/server/srcRepository: juspay/superposition
Length of output: 2458
🏁 Script executed:
sed -n '490,510p' tooling/lsp/server/src/diagnostics.rsRepository: juspay/superposition
Length of output: 859
🏁 Script executed:
rg -n 'ServerCapabilities|position_encoding' tooling/lsp/server/src --context=5Repository: juspay/superposition
Length of output: 1007
🏁 Script executed:
rg -n 'struct ServerCapabilities|position_encoding' tooling/lsp/server/src/backend.rs -A 30Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
head -n 30 tooling/lsp/server/src/backend.rsRepository: juspay/superposition
Length of output: 733
🏁 Script executed:
cat -n tooling/lsp/server/src/backend.rs | sed -n '31,56p'Repository: juspay/superposition
Length of output: 1356
🏁 Script executed:
rg -n 'position_encoding' . --type rust | head -20Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
cat Cargo.toml | grep -A 3 'tower_lsp'Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
find . -name 'Cargo.toml' -exec grep -l 'tower_lsp' {} \; | head -5Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
cat tooling/lsp/server/Cargo.toml | grep -A 2 'tower_lsp'Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
python3 << 'EOF'
import subprocess
import json
# Search for ServerCapabilities definition or documentation
result = subprocess.run(['rg', '-n', 'pub struct ServerCapabilities|pub.*position_encoding', 'tooling/lsp/server/src', '-A', '2'],
capture_output=True, text=True)
print("ServerCapabilities search:")
print(result.stdout)
print(result.stderr)
# Also search in Cargo.lock if it exists
result2 = subprocess.run(['rg', 'tower_lsp', 'Cargo.lock', '-A', '1'],
capture_output=True, text=True)
print("\ntower_lsp in Cargo.lock:")
print(result2.stdout if result2.stdout else "Not found or no Cargo.lock")
EOFRepository: juspay/superposition
Length of output: 145
🏁 Script executed:
find tooling -name 'Cargo.toml' -type fRepository: juspay/superposition
Length of output: 130
🏁 Script executed:
cat tooling/lsp/Cargo.toml 2>/dev/null || cat tooling/lsp/server/Cargo.toml 2>/dev/null || echo "File not found"Repository: juspay/superposition
Length of output: 560
🏁 Script executed:
rg 'use tower_lsp::lsp_types' tooling/lsp/server/src/ -A 2Repository: juspay/superposition
Length of output: 906
🌐 Web query:
tower-lsp 0.20 position_encoding ServerCapabilities
💡 Result:
In tower-lsp 0.20, ServerCapabilities is the lsp-types struct, and it has:
position_encoding: Option<PositionEncodingKind>— “the position encoding the server picked from the encodings offered by the client viageneral.positionEncodings”. If the client didn’t offer anything, the only valid value is UTF‑16; if you omit the field it also defaults to UTF‑16. [1]
You read what the client supports from the initialize request (InitializeParams { capabilities: ClientCapabilities, ... }). [2] Concretely, the encodings (if any) come from params.capabilities.general.position_encodings.
Example (pick UTF‑8 if the client offers it, else UTF‑16), then advertise it via ServerCapabilities.position_encoding:
use tower_lsp::lsp_types::{
InitializeParams, InitializeResult, ServerCapabilities, PositionEncodingKind,
};
async fn initialize(params: InitializeParams) -> tower_lsp::jsonrpc::Result<InitializeResult> {
let client_offered = params
.capabilities
.general
.as_ref()
.and_then(|g| g.position_encodings.as_deref())
.unwrap_or_default();
let chosen = if client_offered.contains(&PositionEncodingKind::UTF8) {
PositionEncodingKind::UTF8
} else {
PositionEncodingKind::UTF16
};
Ok(InitializeResult {
capabilities: ServerCapabilities {
position_encoding: Some(chosen),
..Default::default()
},
..Default::default()
})
}This is the same general negotiation approach used by real servers (e.g., rust-analyzer iterates the client’s general.position_encodings and falls back to UTF‑16). [3]
Sources: [1] [2] [3]
🏁 Script executed:
cat -n tooling/lsp/server/src/diagnostics.rs | sed -n '490,498p'Repository: juspay/superposition
Length of output: 466
🏁 Script executed:
cat -n tooling/lsp/server/src/hover.rs | sed -n '15,25p'Repository: juspay/superposition
Length of output: 546
🏁 Script executed:
cat -n tooling/lsp/server/src/utils.rs | sed -n '15,25p'Repository: juspay/superposition
Length of output: 366
Negotiate position encoding in initialize or convert byte-offset helpers to UTF-16.
The codebase treats positions as UTF-8 byte offsets: byte_offset_to_position computes columns as byte distances from line start, hover.rs and utils.rs cast pos.character directly to usize, and diagnostics.rs uses byte offsets throughout. However, initialize does not set ServerCapabilities.position_encoding, leaving the server on the LSP default (UTF-16). On UTF-16 clients with non-ASCII content, hover/completion/diagnostics will misalign.
Add position_encoding negotiation in initialize: read params.capabilities.general.position_encodings, prefer UTF-8 if the client offers it (falling back to UTF-16), and advertise the choice in ServerCapabilities.position_encoding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/server/src/backend.rs` around lines 31 - 56, The initialize
handler currently does not negotiate position encoding, causing
UTF-16/byte-offset mismatches; update async fn initialize(&self, params:
InitializeParams) to inspect params.capabilities.general.position_encodings (or
the equivalent capability path) and choose an encoding: prefer
PositionEncodingKind::UTF8 if the client advertises it, otherwise fall back to
PositionEncodingKind::UTF16; store the chosen encoding in the server state and
set ServerCapabilities.position_encoding to that choice so clients know what the
server uses; ensure callers that rely on byte offsets (e.g.,
byte_offset_to_position, hover.rs, utils.rs, diagnostics.rs) either read the
stored encoding and convert between byte offsets and LSP positions accordingly
or are updated later to expect the negotiated encoding.
Comment on lines
+33
to
+69
| // Parse raw TOML for dimension / config names; failures are tolerated | ||
| // (the file may be mid-edit and syntactically incomplete). | ||
| let raw = toml::from_str::<toml::Table>(text).ok(); | ||
|
|
||
| let items: Vec<CompletionItem> = match ctx { | ||
| CursorContext::ContextInlineTable => raw | ||
| .as_ref() | ||
| .and_then(|t| t.get("dimensions")) | ||
| .and_then(|d| d.as_table()) | ||
| .map(|dims| { | ||
| dims.keys() | ||
| .map(|k| CompletionItem { | ||
| label: k.clone(), | ||
| kind: Some(CompletionItemKind::FIELD), | ||
| detail: Some("dimension".to_string()), | ||
| ..Default::default() | ||
| }) | ||
| .collect() | ||
| }) | ||
| .unwrap_or_default(), | ||
|
|
||
| CursorContext::OverrideKey => raw | ||
| .as_ref() | ||
| .and_then(|t| t.get("default-configs")) | ||
| .and_then(|d| d.as_table()) | ||
| .map(|configs| { | ||
| configs | ||
| .keys() | ||
| .map(|k| CompletionItem { | ||
| label: k.clone(), | ||
| kind: Some(CompletionItemKind::FIELD), | ||
| detail: Some("config key".to_string()), | ||
| ..Default::default() | ||
| }) | ||
| .collect() | ||
| }) | ||
| .unwrap_or_default(), |
Contributor
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
git ls-files | grep -E "tooling/lsp/server/src/completions.rs"Repository: juspay/superposition
Repository: juspay/superposition
Exit code: 0
stdout:
tooling/lsp/server/src/completions.rs
Script executed:
wc -l tooling/lsp/server/src/completions.rsRepository: juspay/superposition
Repository: juspay/superposition
Exit code: 0
stdout:
162 tooling/lsp/server/src/completions.rs
Script executed:
cat -n tooling/lsp/server/src/completions.rs | head -100Repository: juspay/superposition
Repository: juspay/superposition
Exit code: 0
stdout:
1 use tower_lsp::lsp_types::*;
2
3 use crate::utils;
4
5 /// Context of the cursor within a SuperTOML document.
6 #[derive(Debug, PartialEq)]
7 enum CursorContext {
8 /// Inside the `_context_ = { … }` inline table of an `[[overrides]]` block.
9 /// → suggest declared dimension names.
10 ContextInlineTable,
11 /// On a key line after `_context_` in an `[[overrides]]` block.
12 /// → suggest declared default-config keys.
13 OverrideKey,
14 /// On a `type = …` value in a `[dimensions]` entry.
15 /// → suggest REGULAR / LOCAL_COHORT:… / REMOTE_COHORT:…
16 DimensionType,
17 /// Cursor position not in a recognised completion context.
18 Unknown,
19 }
20
21 /// Produce completion items for the given cursor position.
22 pub fn compute(text: &str, pos: Position) -> Option<CompletionResponse> {
23 // Don't provide completions if cursor is inside a comment
24 if utils::is_inside_comment(text, pos) {
25 return None;
26 }
27
28 let lines: Vec<&str> = text.lines().collect();
29 let line_num = pos.line as usize;
30
31 let ctx = determine_cursor_context(&lines, line_num);
32
33 // Parse raw TOML for dimension / config names; failures are tolerated
34 // (the file may be mid-edit and syntactically incomplete).
35 let raw = toml::from_str::<toml::Table>(text).ok();
36
37 let items: Vec<CompletionItem> = match ctx {
38 CursorContext::ContextInlineTable => raw
39 .as_ref()
40 .and_then(|t| t.get("dimensions"))
41 .and_then(|d| d.as_table())
42 .map(|dims| {
43 dims.keys()
44 .map(|k| CompletionItem {
45 label: k.clone(),
46 kind: Some(CompletionItemKind::FIELD),
47 detail: Some("dimension".to_string()),
48 ..Default::default()
49 })
50 .collect()
51 })
52 .unwrap_or_default(),
53
54 CursorContext::OverrideKey => raw
55 .as_ref()
56 .and_then(|t| t.get("default-configs"))
57 .and_then(|d| d.as_table())
58 .map(|configs| {
59 configs
60 .keys()
61 .map(|k| CompletionItem {
62 label: k.clone(),
63 kind: Some(CompletionItemKind::FIELD),
64 detail: Some("config key".to_string()),
65 ..Default::default()
66 })
67 .collect()
68 })
69 .unwrap_or_default(),
70
71 CursorContext::DimensionType => {
72 let dim_names: Vec<String> = raw
73 .as_ref()
74 .and_then(|t| t.get("dimensions"))
75 .and_then(|d| d.as_table())
76 .map(|dims| dims.keys().cloned().collect())
77 .unwrap_or_default();
78
79 let mut items = vec![CompletionItem {
80 label: "REGULAR".to_string(),
81 kind: Some(CompletionItemKind::ENUM_MEMBER),
82 detail: Some("Standard dimension (default)".to_string()),
83 ..Default::default()
84 }];
85
86 for dim in &dim_names {
87 items.push(CompletionItem {
88 label: format!("LOCAL_COHORT:{}", dim),
89 kind: Some(CompletionItemKind::ENUM_MEMBER),
90 detail: Some(format!("Local cohort of dimension '{}'", dim)),
91 ..Default::default()
92 });
93 items.push(CompletionItem {
94 label: format!("REMOTE_COHORT:{}", dim),
95 kind: Some(CompletionItemKind::ENUM_MEMBER),
96 detail: Some(format!("Remote cohort of dimension '{}'", dim)),
97 ..Default::default()
98 });
99 }
100 items
Script executed:
cat -n tooling/lsp/server/src/completions.rs | tail -70Repository: juspay/superposition
Repository: juspay/superposition
Exit code: 0
stdout:
93 items.push(CompletionItem {
94 label: format!("REMOTE_COHORT:{}", dim),
95 kind: Some(CompletionItemKind::ENUM_MEMBER),
96 detail: Some(format!("Remote cohort of dimension '{}'", dim)),
97 ..Default::default()
98 });
99 }
100 items
101 }
102
103 CursorContext::Unknown => return None,
104 };
105
106 if items.is_empty() {
107 None
108 } else {
109 Some(CompletionResponse::Array(items))
110 }
111 }
112
113 /// Determine what kind of completion is appropriate at `line_num`.
114 ///
115 /// Strategy: scan from the top of the file to `line_num`, tracking which
116 /// section header (`[default-configs]`, `[dimensions]`, `[[overrides]]`) was
117 /// most recently seen and whether `_context_` has appeared in the current
118 /// overrides block.
119 fn determine_cursor_context(lines: &[&str], line_num: usize) -> CursorContext {
120 let up_to = (line_num + 1).min(lines.len());
121
122 let mut in_overrides = false;
123 let mut context_seen = false;
124 let mut in_dimensions = false;
125
126 for line in &lines[..up_to] {
127 let trimmed = line.trim();
128
129 if trimmed == "[[overrides]]" {
130 in_overrides = true;
131 in_dimensions = false;
132 context_seen = false;
133 } else if trimmed == "[dimensions]" {
134 in_dimensions = true;
135 in_overrides = false;
136 context_seen = false;
137 } else if trimmed.starts_with('[') {
138 // Any other section header ([default-configs], etc.)
139 in_overrides = false;
140 in_dimensions = false;
141 context_seen = false;
142 } else if in_overrides && trimmed.starts_with("_context_") {
143 context_seen = true;
144 }
145 }
146
147 let current_line = lines.get(line_num).map(|l| l.trim()).unwrap_or("");
148
149 if in_overrides {
150 if current_line.contains("_context_") {
151 CursorContext::ContextInlineTable
152 } else if context_seen {
153 CursorContext::OverrideKey
154 } else {
155 CursorContext::Unknown
156 }
157 } else if in_dimensions && current_line.contains("type") {
158 CursorContext::DimensionType
159 } else {
160 CursorContext::Unknown
161 }
162 }
Script executed:
grep -n "determine_cursor_context" tooling/lsp/server/src/completions.rsRepository: juspay/superposition
Repository: juspay/superposition
Exit code: 0
stdout:
31: let ctx = determine_cursor_context(&lines, line_num);
119:fn determine_cursor_context(lines: &[&str], line_num: usize) -> CursorContext {
All completion contexts disappear on the first parse error.
The _context_, override-key, and dimension-type branches all depend on raw, which is assigned from toml::from_str(text).ok() (line 35). When the parser encounters a syntax error anywhere in the file, raw becomes None, causing all three match arms to return empty Vec via unwrap_or_default(). The function then returns None for empty items (line 106–110). This is problematic during active editing when the file may be temporarily incomplete; completions should remain available by extracting dimension and config names without requiring a fully-valid parse. The inline comment claims "failures are tolerated," but no fallback mechanism exists to extract partial data from an incomplete TOML structure.
Comment on lines
+143
to
+145
| // Middle segments are sub-nesting (keys that lead to inline tables) | ||
| let sub_nesting = &segments[1..segments.len().saturating_sub(1)]; | ||
|
|
Contributor
There was a problem hiding this comment.
Nested TOML paths are only resolved one level deep.
sub_nesting collects every intermediate segment, but these resolvers only use sub_nesting[0]. A key like dimensions.peak_hours.schema.type will therefore stop at the peak_hours line and return the first type = on that line, not the nested schema field that actually failed validation.
Also applies to: 223-253, 410-438
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/server/src/diagnostics.rs` around lines 143 - 145, The code
slices intermediate segments into sub_nesting but only uses sub_nesting[0], so
nested TOML paths like dimensions.peak_hours.schema.type stop one level early;
update the resolver that references sub_nesting to iteratively traverse each
segment in sub_nesting (e.g., starting from the root table/value, loop over each
segment name, descend into its table/inline table or array element as
appropriate, and update the current context) rather than indexing [0], and fall
back gracefully when a key is missing; apply the same change to the other
resolver occurrences noted (the blocks around the other sub_nesting uses).
Comment on lines
+20
to
+23
| // A best-effort parse; we still try to provide hover even when the file | ||
| // has minor errors by working with the raw table. | ||
| let raw: toml::Table = toml::from_str(text).ok()?; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n tooling/lsp/server/src/hover.rs | head -60Repository: juspay/superposition
Length of output: 2473
🏁 Script executed:
# Find the toml dependency specification
rg "toml\s*=" Cargo.toml | head -20Repository: juspay/superposition
Length of output: 183
🏁 Script executed:
# Check if there are any partial parsing or error-tolerant parsing attempts elsewhere
rg "toml::" tooling/lsp/server/src/ --type rustRepository: juspay/superposition
Length of output: 244
🏁 Script executed:
# Check how completions.rs handles the parse result
cat -n tooling/lsp/server/src/completions.rs | head -80Repository: juspay/superposition
Length of output: 3519
🏁 Script executed:
# Double-check the toml crate version and its behavior
rg "0.8.8" --type tomlRepository: juspay/superposition
Length of output: 2776
Hover stops working as soon as the buffer has any TOML syntax error.
toml::from_str(text).ok()? is not best-effort parsing; one syntax error anywhere makes this return None and disables hover entirely. The comment claiming "best-effort parse" contradicts the implementation. Compare with completions.rs, which handles the same parser by storing the Option without early exit, allowing graceful degradation during editing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/server/src/hover.rs` around lines 20 - 23, The code currently
does a hard early return using toml::from_str(text).ok()? which disables hover
on any TOML syntax error; change this to a best-effort approach by capturing the
parse result as an Option (e.g. let raw: Option<toml::Table> =
toml::from_str(text).ok();) instead of using the ? operator, and update
subsequent uses of raw in hover.rs to handle None gracefully (mirror the pattern
used in completions.rs) so hover can still operate using the raw buffer or
partial data when parsing fails.
Comment on lines
+132
to
+163
| // Try to find the binary in common locations | ||
| const possiblePaths: string[] = []; | ||
|
|
||
| // Check workspace folders | ||
| if (workspace.workspaceFolders) { | ||
| for (const folder of workspace.workspaceFolders) { | ||
| const folderPath = folder.uri.fsPath; | ||
| possiblePaths.push( | ||
| path.join(folderPath, 'target', 'debug', 'supertoml-analyzer'), | ||
| path.join(folderPath, 'target', 'release', 'supertoml-analyzer'), | ||
| // Also check parent directory in case workspace is a subdirectory | ||
| path.join(folderPath, '..', 'target', 'debug', 'supertoml-analyzer'), | ||
| path.join(folderPath, '..', 'target', 'release', 'supertoml-analyzer') | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Check relative to extension path (development mode) | ||
| const extensionRoot = path.dirname(path.dirname(context.extensionPath)); | ||
| possiblePaths.push( | ||
| path.join(extensionRoot, 'target', 'debug', 'supertoml-analyzer'), | ||
| path.join(extensionRoot, 'target', 'release', 'supertoml-analyzer') | ||
| ); | ||
|
|
||
| // Try each path | ||
| for (const p of possiblePaths) { | ||
| const resolvedPath = path.resolve(p); | ||
| if (fs.existsSync(resolvedPath)) { | ||
| outputChannel.appendLine(`Found server at: ${resolvedPath}`); | ||
| return resolvedPath; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Windows compatibility issue in binary path discovery.
The possible paths array doesn't include the .exe extension for Windows. While line 166 correctly handles this for the PATH fallback, the workspace and extension path searches will fail on Windows.
🐛 Proposed fix
function getServerPath(context: ExtensionContext): string {
const config = workspace.getConfiguration('superTOML');
const configuredPath = config.get<string>('server.path', '');
+ const binaryName = process.platform === 'win32' ? 'supertoml-analyzer.exe' : 'supertoml-analyzer';
// If a path is explicitly configured, use it
if (configuredPath && configuredPath.trim() !== '') {
outputChannel.appendLine(`Using configured server path: ${configuredPath}`);
return configuredPath;
}
// Try to find the binary in common locations
const possiblePaths: string[] = [];
// Check workspace folders
if (workspace.workspaceFolders) {
for (const folder of workspace.workspaceFolders) {
const folderPath = folder.uri.fsPath;
possiblePaths.push(
- path.join(folderPath, 'target', 'debug', 'supertoml-analyzer'),
- path.join(folderPath, 'target', 'release', 'supertoml-analyzer'),
+ path.join(folderPath, 'target', 'debug', binaryName),
+ path.join(folderPath, 'target', 'release', binaryName),
// Also check parent directory in case workspace is a subdirectory
- path.join(folderPath, '..', 'target', 'debug', 'supertoml-analyzer'),
- path.join(folderPath, '..', 'target', 'release', 'supertoml-analyzer')
+ path.join(folderPath, '..', 'target', 'debug', binaryName),
+ path.join(folderPath, '..', 'target', 'release', binaryName)
);
}
}
// Check relative to extension path (development mode)
const extensionRoot = path.dirname(path.dirname(context.extensionPath));
possiblePaths.push(
- path.join(extensionRoot, 'target', 'debug', 'supertoml-analyzer'),
- path.join(extensionRoot, 'target', 'release', 'supertoml-analyzer')
+ path.join(extensionRoot, 'target', 'debug', binaryName),
+ path.join(extensionRoot, 'target', 'release', binaryName)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Try to find the binary in common locations | |
| const possiblePaths: string[] = []; | |
| // Check workspace folders | |
| if (workspace.workspaceFolders) { | |
| for (const folder of workspace.workspaceFolders) { | |
| const folderPath = folder.uri.fsPath; | |
| possiblePaths.push( | |
| path.join(folderPath, 'target', 'debug', 'supertoml-analyzer'), | |
| path.join(folderPath, 'target', 'release', 'supertoml-analyzer'), | |
| // Also check parent directory in case workspace is a subdirectory | |
| path.join(folderPath, '..', 'target', 'debug', 'supertoml-analyzer'), | |
| path.join(folderPath, '..', 'target', 'release', 'supertoml-analyzer') | |
| ); | |
| } | |
| } | |
| // Check relative to extension path (development mode) | |
| const extensionRoot = path.dirname(path.dirname(context.extensionPath)); | |
| possiblePaths.push( | |
| path.join(extensionRoot, 'target', 'debug', 'supertoml-analyzer'), | |
| path.join(extensionRoot, 'target', 'release', 'supertoml-analyzer') | |
| ); | |
| // Try each path | |
| for (const p of possiblePaths) { | |
| const resolvedPath = path.resolve(p); | |
| if (fs.existsSync(resolvedPath)) { | |
| outputChannel.appendLine(`Found server at: ${resolvedPath}`); | |
| return resolvedPath; | |
| } | |
| } | |
| function getServerPath(context: ExtensionContext): string { | |
| const config = workspace.getConfiguration('superTOML'); | |
| const configuredPath = config.get<string>('server.path', ''); | |
| const binaryName = process.platform === 'win32' ? 'supertoml-analyzer.exe' : 'supertoml-analyzer'; | |
| // If a path is explicitly configured, use it | |
| if (configuredPath && configuredPath.trim() !== '') { | |
| outputChannel.appendLine(`Using configured server path: ${configuredPath}`); | |
| return configuredPath; | |
| } | |
| // Try to find the binary in common locations | |
| const possiblePaths: string[] = []; | |
| // Check workspace folders | |
| if (workspace.workspaceFolders) { | |
| for (const folder of workspace.workspaceFolders) { | |
| const folderPath = folder.uri.fsPath; | |
| possiblePaths.push( | |
| path.join(folderPath, 'target', 'debug', binaryName), | |
| path.join(folderPath, 'target', 'release', binaryName), | |
| // Also check parent directory in case workspace is a subdirectory | |
| path.join(folderPath, '..', 'target', 'debug', binaryName), | |
| path.join(folderPath, '..', 'target', 'release', binaryName) | |
| ); | |
| } | |
| } | |
| // Check relative to extension path (development mode) | |
| const extensionRoot = path.dirname(path.dirname(context.extensionPath)); | |
| possiblePaths.push( | |
| path.join(extensionRoot, 'target', 'debug', binaryName), | |
| path.join(extensionRoot, 'target', 'release', binaryName) | |
| ); | |
| // Try each path | |
| for (const p of possiblePaths) { | |
| const resolvedPath = path.resolve(p); | |
| if (fs.existsSync(resolvedPath)) { | |
| outputChannel.appendLine(`Found server at: ${resolvedPath}`); | |
| return resolvedPath; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/vscode-extension/src/extension.ts` around lines 132 - 163, The
workspace/extension path discovery builds possiblePaths without Windows
executables, so on Windows the searches miss the real binary; update the code
that populates possiblePaths (the blocks referencing workspace.workspaceFolders
and extensionRoot) to also include the '.exe' variant when process.platform ===
'win32' (or push both with and without '.exe'), ensuring the same
Windows-handling used in the PATH fallback is applied to the checks that use
possiblePaths and resolvedPath.
| @@ -0,0 +1 @@ | |||
| (pair (bare_key) "=" (_) @redact) | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare redaction patterns in-repo and confirm current convention before merging.
fd redactions.scm -t f | xargs -I{} sh -c 'echo "=== {} ==="; sed -n "1,120p" "{}"'
rg -n --type-add 'scm:*.scm' --type scm -C2 '#match\\?|@redact|\\(pair' tooling/lspRepository: juspay/superposition
Length of output: 280
🏁 Script executed:
#!/bin/bash
# Find all .scm files in the repository
fd "\.scm$" -t f
# Search for `@redact` and similar patterns more carefully
rg "@redact" -n
# Look for `#match` patterns (which indicate conditional matching)
rg "#match" -n -C 2
# Check for other language definition files
fd redactions\\.scm\\|language\\. -t f | head -20Repository: juspay/superposition
Length of output: 3839
🏁 Script executed:
#!/bin/bash
# Get more context about the file and its surrounding directory
cd tooling/lsp/zed-extension/languages/superTOML 2>/dev/null && ls -la
cat highlights.scm 2>/dev/null || echo "No highlights.scm found"Repository: juspay/superposition
Length of output: 1223
🏁 Script executed:
#!/bin/bash
# Search for TOML-related language definitions in repo
fd -e scm -e json -t f | xargs rg -l "toml|TOML" | head -10Repository: juspay/superposition
Length of output: 9810
🏁 Script executed:
#!/bin/bash
# Search for `@redact` usage in the repo and any related documentation
rg "@redact" -B2 -A2
# Look for any documentation about redactions
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "README*" \) | xargs grep -l "redact" 2>/dev/null | head -10
# Check if there's a Zed configuration or schema that explains redactions
find . -name "*.toml" -o -name "*.json" | xargs grep -l "redact" 2>/dev/null | head -5Repository: juspay/superposition
Length of output: 158
🏁 Script executed:
#!/bin/bash
# Look at the entire redactions.scm file and nearby files for context
cat tooling/lsp/zed-extension/languages/superTOML/redactions.scm
# Check the config.toml in the same directory
cat tooling/lsp/zed-extension/languages/superTOML/config.toml
# Look for similar patterns in other Zed language extensions
find tooling/lsp/zed-extension/languages -name "redactions.scm" -o -name "*redact*"Repository: juspay/superposition
Length of output: 624
🏁 Script executed:
#!/bin/bash
# Check if this is a new file or has history
cd tooling/lsp/zed-extension/languages/superTOML && git log --oneline -n 10 redactions.scm 2>&1 || echo "No git history available"
# Search for any Zed documentation or language definition patterns
find . -type f -name "*.md" | xargs grep -i "redact\|sensitive" 2>/dev/null | head -20
# Look at other language extensions in Zed to see if any have redaction patterns
find tooling/lsp/zed-extension/languages -type f -name "*.scm" | xargs grep -l "@redact\|redaction" 2>/dev/nullRepository: juspay/superposition
Length of output: 191
🏁 Script executed:
#!/bin/bash
# Check Zed language extension documentation/examples
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -l "zed.*extension\|language.*server" 2>/dev/null | head -5
# Look at the entire language directory structure
find tooling/lsp/zed-extension/languages -type d | head -20Repository: juspay/superposition
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Read the Zed extension README to understand context
cat tooling/lsp/zed-extension/README.md
# Check git log for the redactions.scm file
git log --oneline -n 20 -- tooling/lsp/zed-extension/languages/superTOML/redactions.scmRepository: juspay/superposition
Length of output: 2732
🏁 Script executed:
#!/bin/bash
# Look at all .scm files in the superTOML directory
ls -la tooling/lsp/zed-extension/languages/superTOML/
# Read each .scm file to understand the patterns
for file in tooling/lsp/zed-extension/languages/superTOML/*.scm; do
echo "=== $(basename $file) ==="
cat "$file"
echo ""
doneRepository: juspay/superposition
Length of output: 1970
🏁 Script executed:
#!/bin/bash
# Search for any Zed-specific documentation or references to `@redact`
rg "redact" --type-list | head -20
# Check git history to see when redactions.scm was added
git log --all -p -- "**/redactions.scm" | head -100
# Look for Zed extension.toml or other config files
find tooling/lsp/zed-extension -name "*.toml" -exec echo "=== {} ===" \; -exec cat {} \;Repository: juspay/superposition
Length of output: 2338
🏁 Script executed:
#!/bin/bash
# Search web context or references to understand Zed redaction feature
# Look for any comments or documentation in extension source
find tooling/lsp/zed-extension/src -type f | xargs cat 2>/dev/null
# Check if there's any reference to redactions in Cargo.toml or extension.toml
cat tooling/lsp/zed-extension/extension.toml 2>/dev/null || echo "No extension.toml"
cat tooling/lsp/zed-extension/Cargo.toml 2>/dev/null | head -50Repository: juspay/superposition
Length of output: 3802
Redaction pattern should be limited to sensitive keys, not all values.
Line 1 currently redacts every TOML pair value. This defeats usability in the editor by masking non-sensitive configuration data. Add a #match? conditional to filter for sensitive keys only, consistent with pattern matching conventions used elsewhere in the project.
Suggested fix
-(pair (bare_key) "=" (_) `@redact`)
+(pair
+ (bare_key) `@key`
+ "="
+ (_) `@redact`
+ (`#match`? `@key` "(?i)^(password|passwd|secret|token|api[_-]?key|private[_-]?key)$"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| (pair (bare_key) "=" (_) @redact) | |
| (pair | |
| (bare_key) `@key` | |
| "=" | |
| (_) `@redact` | |
| (`#match`? `@key` "(?i)^(password|passwd|secret|token|api[_-]?key|private[_-]?key)$")) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/zed-extension/languages/superTOML/redactions.scm` at line 1, The
current S-expression rule (pair (bare_key) "=" (_) `@redact`) redacts all TOML
values; restrict it by adding a `#match`? conditional on the bare_key so only
known sensitive keys are redacted (e.g., "password","secret","api_key","token"
or the project's canonical sensitive-key list). Locate the pattern that uses
bare_key and `@redact` and wrap the value capture with a conditional check (using
`#match`? against the sensitive-key regex or list) so non-sensitive pairs are not
assigned `@redact` while preserving existing token names (bare_key, `@redact`).
Comment on lines
+20
to
+34
| // Check if we already have a cached path | ||
| if let Some(path) = &self.cached_binary_path { | ||
| return Ok(Command { | ||
| command: path.clone(), | ||
| args: Vec::new(), | ||
| env: Default::default(), | ||
| }); | ||
| } | ||
| Ok(Command { | ||
| command: | ||
| "/Users/kartik.gajendra/superposition/target/debug/supertoml-analyzer" | ||
| .to_string(), | ||
| args: Vec::new(), | ||
| env: Default::default(), | ||
| }) |
Contributor
There was a problem hiding this comment.
Don't ship a machine-specific analyzer path.
This only works on /Users/kartik.gajendra/..., so the extension cannot start the language server anywhere else, and the cache branch is never populated. Resolve the binary from the packaged extension/environment at runtime and fail cleanly when it is unavailable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tooling/lsp/zed-extension/src/lib.rs` around lines 20 - 34, Replace the
hard-coded machine-specific path by resolving the analyzer binary at runtime:
remove the literal "/Users/..." string in the Command returned and instead look
up the bundled/packaged binary (via environment/config, PATH lookup, or
extension resource loading) and populate self.cached_binary_path when found; if
resolution fails return an Err (or a clear failure Command) rather than trying
to spawn a non-existent path. Update the code that constructs Command (the block
that currently returns Command { command: "...".to_string(), ... }) to use the
resolved path and ensure cached_binary_path is set so the cache branch is
exercised.
Collaborator
There was a problem hiding this comment.
"/Users/kartik.gajendra/superposition/target/debug/supertoml-analyzer"
.to_string(),
Can we change this ? This isn't gonna work on anywhere else
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the full current content of the lib.rs file
fd "lib.rs" tooling/lsp --exec cat {}
# Check what zed_extension_api version is used and what APIs are available
fd "Cargo.toml" tooling/lsp/zed-extension --exec cat {}
Length of output: 3123
🌐 Web query:
zed_extension_api 0.7 language_server_command binary resolution worktree which
💡 Result:
In zed_extension_api 0.7.x, Extension::language_server_command(...) must return a zed_extension_api::Command whose command field is a path string Zed can execute. The typical patterns are:
- Use a user/system-installed binary (preferred when you don’t bundle an LSP):
- Call
worktree.which("my-lsp"), which “returns the path to the given binary name, if one is present on the$PATH”. - Use the returned absolute path as
Command.command, and usually passenv: worktree.shell_env()so the server inherits the worktree’s shell environment. [1][2]
- Use a binary bundled with the extension:
- Return a path relative to the extension (e.g.
bin/my-lsporserver/my-lsp). - On the host side, Zed resolves what you returned via
path_from_extension(...)before launching the process (so the relative path becomes an absolute path inside the installed extension directory). [3]
So: worktree.which(...) is for resolving executables on the worktree $PATH, while relative command strings are resolved as extension-relative paths by Zed. [1][3]
Sources
[1] https://docs.rs/zed_extension_api/latest/zed_extension_api/struct.Worktree.html
[2] https://docs.rs/zed_extension_api/latest/zed_extension_api/trait.Extension.html
[3] https://readmex.com/en-US/zed-industries/zed/page-6.1822b1e7b-14b2-434c-96d4-a4bfc121f722
@sauraww — agreed, this is a blocker. The fix is already partially written in the commented-out code inside the file — it just needs to be activated. The worktree.which("supertoml-analyzer") API is the correct approach per the zed_extension_api docs: worktree.which("my-lsp") "returns the path to the given binary name, if one is present on the $PATH", and you should also pass env: worktree.shell_env() so the server inherits the worktree's shell environment.
⚠️ The workspace-relativestd::path::Path::new(&path).exists()checks from the commented block should not be restored — Zed extensions run in a WASM sandbox and don't have direct filesystem access that way.
Here's the corrected language_server_command:
fn language_server_command(
&mut self,
_language_server_id: &LanguageServerId,
worktree: &Worktree, // remove leading `_`
) -> Result<Command> {
// Return cached path if already resolved
if let Some(path) = &self.cached_binary_path {
return Ok(Command {
command: path.clone(),
args: Vec::new(),
env: worktree.shell_env(),
});
}
// Resolve from the worktree's $PATH
let binary = worktree
.which("supertoml-analyzer")
.ok_or_else(|| {
"supertoml-analyzer not found in PATH. \
Please build it with `cargo build -p supertoml_lsp` \
and add the binary to your PATH.".to_string()
})?;
self.cached_binary_path = Some(binary.clone());
Ok(Command {
command: binary,
args: Vec::new(),
env: worktree.shell_env(),
})
}Key changes:
- Removed the hard-coded
/Users/kartik.gajendra/...path entirely. worktree.which("supertoml-analyzer")resolves the binary from the user's$PATH— this is the standard pattern for Zed extensions that don't bundle their own LSP binary.cached_binary_pathis now populated after a successful lookup, so the cache branch is actually exercised on subsequent calls.- Returns an
Errwith a clear message when the binary is missing, rather than attempting to spawn a non-existent path. env: worktree.shell_env()passed in both branches so the server inherits the correct shell environment.
🧠 Learnings used
Learnt from: ayushjain17
Repo: juspay/superposition PR: 827
File: crates/superposition_core/src/config.rs:154-154
Timestamp: 2026-01-07T20:38:53.153Z
Learning: Deprecate jsonlogic-based context conditions and migrate to simple key-value pair map conditions across the codebase. Replace jsonlogic::apply usage with superposition_types::apply for condition evaluation. Update all relevant modules (notably Rust files under crates) to use the new approach, remove jsonlogic dependencies where applicable, and run full compilation and tests to catch regressions. Ensure context evaluation logic consistently uses key-value maps and that the architectural change is reflected in unit/integration tests.
Collaborator
Author
There was a problem hiding this comment.
@sauraww yeah made this point during the standup. Review only the LSP. None of the plugins will work without code changes
Contributor
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Datron
force-pushed
the
claude/supertoml-lsp-server-6Mrbi
branch
6 times, most recently
from
6c4aa0d to
1be3a7c
Compare
Adds a new `supertoml_lsp` crate that implements a Language Server
Protocol server for the SuperTOML configuration format used by
Superposition.
Features:
- **Diagnostics** – two-pass validation: raw TOML syntax errors are
reported with accurate byte-span positions (via the `toml` crate's
span API); semantic errors from `parse_toml_config` are mapped to
source ranges by searching for the offending token in the document.
- **Completion** – context-aware suggestions:
- Dimension names inside `_context_ = { … }` inline tables
- Config keys (from `[default-configs]`) on override key lines
- `REGULAR` / `LOCAL_COHORT:<dim>` / `REMOTE_COHORT:<dim>` for
`type = …` fields in `[dimensions]`
- **Hover** – shows position, type, and schema for dimension names;
shows default value and schema for config keys.
The server communicates over stdin/stdout (standard LSP transport) and
is built with `tower-lsp`.
https://claude.ai/code/session_01BMN7G45dz4TsP4GkQYXCW5
Signed-off-by: datron <Datron@users.noreply.github.com>
Datron
force-pushed
the
claude/supertoml-lsp-server-6Mrbi
branch
from
1be3a7c to
beef656
Compare
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
2 participants
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.
Problem
No LSP support for superTOML
Solution
Add LSP server and vscode/nvim plugins for superTOML
Summary by CodeRabbit
New Features
[[context]]blocks with[[overrides]]blocks and_condition_with_context_for improved clarity.Documentation