Repository navigation
Add the Mantid-aware script analysis service - #1
Merged
Merged
Conversation
A stateless HTTP service the FIA script editors can call on a debounce, returning Monaco-shaped diagnostics for a reduction script without running it. Built FROM a pinned Mantid runner tag, because only the interpreter that runs the scripts can answer questions about them. - Warm-up imports the framework in a background thread (the import deadlocks inside an event loop) and gates readiness on a real algorithm having run: Mantid 7's AlgorithmManager reports nothing useful, so readiness creates and deletes a probe workspace instead. - Positions are Monaco's - 1-based lines, 1-based UTF-16 columns, exclusive ends. CPython reports code-point offsets, jedi code-point columns, pyflakes UTF-8 byte offsets: four conventions, and positions.py is the only module that knows them. - compile()'s verdict merged with jedi's error-recovering coverage, so one mistake yields one diagnostic - the cause where it happened rather than the effect where parsing gave up. Ranges are never zero-width because Monaco draws nothing for one. - Framework names come from dir() on the live modules at warm-up, rendered as a generated preamble, and pyflakes reports undefined names over the top. Only star imports of those modelled modules are neutralised: blanking `from isis_powder import *` claims facility names were supplied, which measured as three false positives on a script that runs correctly. - Analysis runs in a forkserver process pool sized from the cgroup CPU quota, and every worker completes a probe analysis before readiness: threads measured 4.6x slower than serial on this CPU-bound work, processes 18x faster than threads, and an unwarmed worker pays jedi's import inside a user's keystroke. - script_lsp is at 100% statement and branch coverage without Mantid installed; the two framework-only functions are excluded with a stated reason.
tests.yml runs the unit suite, ruff and mypy on every push and PR. It needs no Mantid, which is only true because nothing imports the framework at module scope - if that ever changes, the job is the thing that breaks first, deliberately. build-and-push.yml runs on main only: it pushes the image to ghcr.io and pins the resulting digest into this service's staging manifest in ../gitops, the same route the other FIA services use. The image build stays out of PR checks because a Mantid base image does not belong on every branch push - which is why Dockerfile and dependency-pin changes have to be built locally before merging. Actions are pinned by SHA, resolved from their tags rather than copied, and the secrets (WRITE_PACKAGE_TOKEN, GITOPS_STAGING_EDIT_TOKEN) are the ones the sibling publish workflows already use.
README covers what gets reported and what deliberately is not (including why the service goes silent about a script that star-imports code it cannot model), who holds a copy of the wire contract across the three repositories, the coordinate conventions, the configuration, and the measurements that decide the probe budgets, resource requests, request cap and editor debounce. AGENTS.md is the agent-facing counterpart: the hard rules that measurements forced (never import Mantid on a request path, keep analysis off the event loop, never invent a framework name, neutralise rather than delete star imports, no in-process analysis fallback), the corpus privacy policy that keeps facility analysis code out of the repository, and the FIA conventions for branches, commits and pull requests.
'Attach the repo's AI label' reads like something the human doing the merge handles. The one who knows a PR was machine-written is the one who opened it.
This was referenced Nov 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No issue to link: this pull request opens the service repository, which did not exist before it. Happy to re-target it at an issue if one should exist.
Description
Adds
mantid-python-script-lsp: a stateless HTTP service that answers "does this reduction script make sense?" without running it, so the FIA script editors can show real squiggles on Mantid scripts instead of none.It is not a language server — no session, no LSP socket, no document state. Each request carries a script and a client-computed hash; each response carries diagnostics positioned in Monaco's coordinates, plus the hash echoed back so the editor can discard a reply that arrived after the user typed something else.
What is in it
POST /analysereturningparsesplus diagnostics:compile()'s verdict, jedi's error-recovering coverage of what CPython could not reach, and pyflakes undefined-name warnings.dir()overmantid.simpleapi,api,kernel,dataobjects,geometry— 1,777 names). Mantid mints its algorithm functions at import time, so no static tool can see them; committing a name list would let it drift from the pinned base image, and inventing a name would suppress a real warning forever.from mantid.simpleapi import *makes pyflakes stop reporting anything real, so those lines are replaced with equal-length comments and the offsets are undone on the way out. Facility star imports (from isis_powder import *) are left alone on purpose — rewriting them claimed their names were supplied, which measured as three false positives on a script that runs correctly./healthzand/ready, matching the paths every other FIA deployment probes.main, in the shape the other services' publish workflows use.Verification
script_lspwithout Mantid installed (the framework is stubbed). The two functions that genuinely need it are excluded with a stated reason and exercised in the image instead.ruff check,ruff format --check,mypy --strict script_lspclean.MERLIN.py, itsimport *variant and an ×8 size variant all return zero diagnostics; a deliberate typo is caught at its real line and column; a deliberately broken script returns the cause where it happened rather than the effect where the parser gave up./readyin ~3 s: framework, names bound, 8 workers probed.Decisions worth a look while reviewing
severityiserrororwarningand nothing else — a third level would be a grey squiggle nobody can act on.autoreduction-scriptsis deliberately not installed, so facility code cannot take this service down or change its answers.frontend.Follow-ups, not in this PR
../gitopsneedscomponents/mantid-python-script-lsp/envs/staging/…before the publish workflow can go green — it edits that manifest.Made with the assistance of AI.