Clean up stale config (pylint, yapf, copilot, dump.rdb)#174
Merged
Conversation
- .pylintrc, .style.yapf: legacy linter/formatter configs replaced by ruff in #173. - copilot/: AWS Copilot CLI manifest, replaced by Pulumi/ECS in the ord-infrastructure repository. - dump.rdb: accidentally committed Redis snapshot. - README.md, CONTRIBUTING.md: setup.py / pip install instructions rewritten in terms of uv now that #173 has landed. - .gitignore: add .pytest_cache/, .ruff_cache/, .venv/, dump.rdb so these stop showing up in future working trees. Editor-tree `# pylint:` / `# pytype:` comments are intentionally left alone; the editor is excluded from ruff/ty and is being removed in #172. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop the placeholder "How to Deploy" section. - Fix the project layout: api/ is the FastAPI server (not /client), and the description was missing it entirely. Mark editor/ as legacy (being removed in #172). - Tighten setup instructions and folder descriptions; trim filler. - Flag the host-port 5432 collision for `docker compose up`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CONTRIBUTING.md: sweep remaining `ord-schema` references to `ord-interface` (title, issue-tracker URLs, fork instructions). The goals/non-goals readthedocs link is left as-is and rephrased to point at "the broader Open Reaction Database" so it's clear the reference is intentional. - README.md: add a brief Deployment section pointing at the ord-infrastructure repo (Pulumi/ECS) now that the old "How to Deploy" placeholder is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the pre-existing directory patterns consistent with the new ones added in this branch (`.pytest_cache/`, `.ruff_cache/`, `.venv/`) and the already-slashed ones above (`.idea/`, `docs/_build/`, `.vscode/`). Trailing slash restricts a pattern to match directories only, which is what we actually want for `__pycache__`, `*.egg-info`, `.ipynb_checkpoints`, `build`, `dist`, `ketcher`, `standalone`, `node_modules`, and `coverage`. A future file accidentally named one of these wouldn't be silently ignored. `**/.pnp` is left as-is because Yarn 2+ uses `.pnp.cjs` files while Yarn 1 used a `.pnp` directory; the un-slashed form matches both. Verified `git check-ignore` still resolves the existing `.venv/`, `ord_interface.egg-info/`, `ord_interface/__pycache__/` directories through the new rules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skearnes
added a commit
that referenced
this pull request
May 12, 2026
Adopts the README rewrite from PR #174 (project layout corrected, "How to Deploy" placeholder dropped, deployment pointer added, schema-update instructions rewritten in terms of uv) and updates the local-development section to match what actually works on a fresh checkout: - Add `redis` to the prerequisites list. The API uses redis for search-task state; if it isn't running, the search route hangs. - Replace the broken `fastapi dev main.py --port=5000` invocation with `uv run uvicorn ord_interface.api.main:app --port 5000 --reload`. `fastapi dev` requires the `fastapi[standard]` extra which isn't in our deps. - Add a `redis-server --daemonize yes` step to the Local option. - Note that the conda env from step 1 must stay activated so the testing.postgresql shells can find initdb on PATH. - Flag the macOS port-5000 collision with AirPlay Receiver and document the workaround (bind/visit via 127.0.0.1, or disable the service in System Settings). - Clarify the Ketcher extraction step so users end up with the right src/ketcher/static + templates layout the build expects. - Mention `npm run dev` / `npm run preview` aliases now that the SPA is on Vite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bdeadman
approved these changes
May 13, 2026
skearnes
added a commit
that referenced
this pull request
May 13, 2026
* app: migrate SPA from @vue/cli to Vite @vue/cli is in maintenance mode and pinned by transitive dependencies that won't ever be patched without a major-version downgrade of the toolchain. Vite is the official Vue successor and resolves the entire class of npm advisories the SPA has been carrying. Toolchain changes: - Replace @vue/cli-service ~5.0.0 and friends with vite ^6.4.2 + @vitejs/plugin-vue. Drops 768 transitive packages from npm install. - Replace the chainWebpack copy step in vue.config.js (ketcher templates → dist/templates) with vite-plugin-static-copy. Same output layout. - Move public/index.html to app/index.html (Vite serves index.html from the project root) and add the explicit `<script type="module" src="/src/main.js">` tag that Vite expects. - Replace process.env.BASE_URL with import.meta.env.BASE_URL in router/index.js (the only env reference in src/). - Rename .eslintrc.js → .eslintrc.cjs because package.json now sets "type": "module"; drop the @babel/eslint-parser parser (espree handles ES2022 fine for the source style here). - Drop babel.config.js (no longer used) and core-js (vite/esbuild honors browserslist without auto-polyfilling). - Bump eslint 7 → 8.57 and eslint-plugin-vue 8 → 9. Configuration choices preserved: - Dev server still binds port 8080 (matches old vue-cli default). - `npm run serve` still works as before; added `npm run dev` and `npm run preview` as Vite-conventional aliases. - `@/*` import alias preserved in vite.config.js + jsconfig.json. - Vite's resolver is configured with the same implicit extensions the old webpack resolver allowed, so existing extension-less imports throughout the codebase keep working without churn. Verified locally: - `npm install` produces 269 packages, 0 vulnerabilities (was 1037 packages, 23 dependabot alerts). - `npm run build` produces a working `dist/` (index.html + assets/* + templates/* + favicon + img/). Build time ~5s vs ~16s with vue-cli. - `npm run serve` boots at http://localhost:8080/ in 156ms; / returns 200 and /src/main.js is transformed correctly. - `npm run preview` (production preview) serves dist/, including /templates/library.svg (2.5MB) and /templates/fg.sdf (30KB). - `npm run lint` passes (one pre-existing unused-var warning in src/utils/amount.js). Dockerfile is unchanged: it still calls `npm install && npm run build` and copies `dist/` to nginx's web root. nginx's SPA `try_files` config doesn't care about asset paths, so the new `assets/` directory works in place of vue-cli's `js/`+`css/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: replace clang-format with prettier clang-format treats .vue files as C-like and produces nonsense results. Prettier is the standard JS/TS/Vue formatter and is already what ord-app uses, so adopting it here keeps formatting consistent between the two repos. - .github/workflows/checks.yml: drop the apt-install + `find ... -exec clang-format` step; add a setup-node step and run `npm run format:check` instead. - app/package.json: add `prettier` as a dev dep with `format` and `format:check` scripts mirroring ord-app's setup. - app/.prettierrc.json: copied verbatim from open-reaction-database/ord-app/ui/.prettierrc.json so the two SPAs produce identical output. - app/.prettierignore: skip generated/vendored trees (dist/, node_modules/, public/, src/ketcher/, package-lock.json). This commit only wires up the tool; the one-time `prettier --write` pass follows in the next commit so the actual formatting churn is trivially separable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: one-time `prettier --write .` pass Applies the prettier config landed in the previous commit to the existing SPA tree. No behavioral change — only whitespace, quote style, trailing commas, and Vue attribute layout. Verified afterward: - `npm run format:check` reports "All matched files use Prettier code style". - `npm run build` still produces a working `dist/` in ~5s. - `npm run lint` reports only the pre-existing unused-var warning in src/utils/amount.js. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * pre-commit: add prettier hook for the SPA Mirrors the ruff-check/ruff-format setup we have for Python: catches formatting issues before they make it to CI. Implemented as a local hook running `npx --no-install prettier --check .` inside app/, so the hook version is whatever package-lock.json pins — no separate mirrored repo to keep in sync. Verified locally with `pre-commit run prettier --all-files`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: migrate sass @import to @use ... as * Sass @import is deprecated and will be removed in Dart Sass 3.0.0. Every build was emitting warnings like: DEPRECATION WARNING [import]: Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0. Mechanical migration of the 36 affected lines across 24 files: @import '@/styles/vars' → @use '@/styles/vars' as * @import '@/styles/vars.sass' → @use '@/styles/vars' as * @import "@/styles/vars" → @use '@/styles/vars' as * @import "./vars" → @use './vars' as * @import @/styles/vars → @use '@/styles/vars' as * @import "../../styles/vars" → @use '../../styles/vars' as * Plain-CSS @imports (Google Fonts url() in App.vue and the Ketcher .css in MainKetcher.vue) are left as-is — Sass treats those as CSS pass-through and doesn't deprecate them. The `as *` clause keeps every existing variable (`$lightgrey`, `$medgrey`, `$linkblue`, etc.) in the global namespace, so no $varname references in the consuming files need to change. One nit fix: in App.vue the `@use` was placed after an `@import url('...')` line, and Sass requires `@use` to be the first rule in a file. Swapped the two lines. Verified: `npm run build` produces a working dist/ in ~5s with zero sass deprecation warnings; format:check and lint stay clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: unscope Ketcher CSS + drop empty button selectors Two small bug fixes surfaced while testing the Vite build: 1. MainKetcher.vue had `<style lang="sass" scoped>` wrapping the Ketcher CSS import. Vue's scoped CSS appends `[data-v-HASH]` to every selector — for Ketcher that's ~1300 rules — but Ketcher builds its own DOM inside `#root` at runtime via JavaScript and those dynamically-created elements don't carry the scope attribute, so none of the ~1300 rules match anything Ketcher actually renders. Result: Ketcher loads but looks "really weird and not usable". Drop `scoped` and add a comment explaining why it has to stay off. 2. SearchResults.vue (both search/ and dataset-view/ flavors) had a dangling `button` selector with no rules under it at the end of `.action-button-holder`. Sass warns about this on every build: "This selector doesn't have any properties and won't be rendered." Just drop the empty line. Verified locally: clean `npm run build` (no warnings), prettier and lint stay clean, and Ketcher CSS no longer carries `[data-v-*]` attributes in the dev-server output (`grep -c data-v-469446ff` went from 1325 → 0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: short-circuit /search when no filters are set The Vue component was firing `/api/submit_query` with an empty query string on first load (the watch on $route.query fires in mounted() regardless of params). The backend rejects that with `ValueError: No query parameters were specified.`, and the frontend then polls forever waiting for a result that's never going to come — manifesting as a stuck spinner on /search. The original codebase had a `RandomSampleQuery(100)` fallback in the old Flask client (commit 6886b0a) that masked this on the backend; the FastAPI rewrite removed it intentionally, so the frontend is now the right place to handle the empty-filter state. Changes: - getSearchResults() bails early if window.location.search is empty: set searchResults=[], loading=false, return without hitting the API. - Template adds a separate empty-state message for the "no query yet" case ("Configure the filters on the left to search the database.") vs. the "search ran, nothing matched" case (existing message). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * README: bring in the cleanup-pass refresh + local-stack fixes Adopts the README rewrite from PR #174 (project layout corrected, "How to Deploy" placeholder dropped, deployment pointer added, schema-update instructions rewritten in terms of uv) and updates the local-development section to match what actually works on a fresh checkout: - Add `redis` to the prerequisites list. The API uses redis for search-task state; if it isn't running, the search route hangs. - Replace the broken `fastapi dev main.py --port=5000` invocation with `uv run uvicorn ord_interface.api.main:app --port 5000 --reload`. `fastapi dev` requires the `fastapi[standard]` extra which isn't in our deps. - Add a `redis-server --daemonize yes` step to the Local option. - Note that the conda env from step 1 must stay activated so the testing.postgresql shells can find initdb on PATH. - Flag the macOS port-5000 collision with AirPlay Receiver and document the workaround (bind/visit via 127.0.0.1, or disable the service in System Settings). - Clarify the Ketcher extraction step so users end up with the right src/ketcher/static + templates layout the build expects. - Mention `npm run dev` / `npm run preview` aliases now that the SPA is on Vite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: bump ESLint 7→9 and migrate to flat config All six npm-install deprecation warnings on a fresh checkout chained out of eslint@8.57.1: npm WARN deprecated rimraf@3.0.2 … npm WARN deprecated inflight@1.0.6 … npm WARN deprecated glob@7.2.3 … npm WARN deprecated @humanwhocodes/config-array@0.13.0 … npm WARN deprecated @humanwhocodes/object-schema@2.0.3 … npm WARN deprecated eslint@8.57.1 … `npm ls` traced all of them to a single chain: `eslint@8` → `file-entry-cache@6` → `flat-cache@3` → `rimraf@3` → `glob@7` → `inflight@1`, plus eslint 8's own `@humanwhocodes/*` deps which were renamed to `@eslint/*` in v9. Migrate to ESLint 9 + flat config: - Bump `eslint` ^8.57.1 → ^9.20.0 and add `@eslint/js` and `globals` as devDeps. - Delete `.eslintrc.cjs` and `.eslintignore`; add `eslint.config.js` (flat config). `eslint-plugin-vue@9.33.0` exposes the Vue-3 essential ruleset at `vue.configs['flat/essential']`. - Move the `ignores` list (`dist/`, `node_modules/`, `src/ketcher/`) into the flat config; ESLint 9 no longer reads `.eslintignore`. One latent bug surfaced by ESLint 9's `no-constant-binary-expression` rule, in MainSelectedSet.vue: return [this.$route.query.reaction_id] || []; The array literal is always truthy, so `|| []` is dead code. Dropped it; behavior is unchanged (the existing call site already always got the LHS branch). Verified locally: fresh `npm install` produces 269 packages with **zero deprecation warnings** and **zero vulnerabilities**; lint, format:check, and build all stay clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: properly normalize reactionIds for the selected-set view The previous commit dropped `|| []` because the LHS was always a non-empty array literal (lint error). But the underlying intent was correct: when `?reaction_id=…` is missing, the computed `reactionIds` should be `[]` so the "Download Reaction Set" button disables itself and the backend POST doesn't ship `[null]`. vue-router gives back three different shapes for query params: ?<none> -> undefined ?reaction_id=foo -> "foo" ?reaction_id=foo&reaction_id=bar -> ["foo", "bar"] Normalize all three via `[value ?? []].flat()` so length reflects the true count in every case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: address review feedback on PR #176 Three small follow-ups to the review: 1. Drop the `resolve.extensions` workaround in vite.config.js. The review pointed out Vite officially discourages including `.vue` in `extensions`, since implicit extensions can confuse IDE/TS tooling. Instead, do a one-shot pass adding `.vue` to all 50 extension-less imports across 16 files in app/src; Vite's default resolver then handles everything cleanly without an opt-in. 2. Pin the dev-server proxy target to `127.0.0.1` instead of `0.0.0.0`. On macOS, `localhost`/`0.0.0.0` resolve via IPv6 first and hit Apple's AirPlay Receiver (which also listens on `*:5000`) instead of uvicorn (IPv4-only). Pinning to `127.0.0.1` dodges the ambiguity entirely. Also expand the Ketcher comment to note that the v2.5.1 build is self-contained — if Ketcher is ever updated to a chunked build, the static-copy targets need to grow. 3. Fix the always-false `NODE_ENV === 'production'` check in eslint.config.js. ESLint never sets NODE_ENV, so `no-console` and `no-debugger` were never warning. Switch to `process.env.CI` which GitHub Actions sets — verified: locally lints 7 warnings, CI mode lints 17 (the 10 extra are the previously-silent console statements). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * app: fix /search hang when the route changes via $router.push Symptom: clicking "Search" from the filters panel left the spinner running forever, but refreshing the page made the same query complete quickly. Root cause: the polling loop that handles `fetch_query_result` returning 202 (task pending) was only set up inside `mounted()`. When the user submits the filter form, `updateSearchOptions` calls `this.$router.push({name: 'search', query: this.searchParams})`, which navigates within the same component. Vue Router reuses the mounted component, so `mounted()` does NOT re-fire — only the watch on `$route.query` does. The watch was calling `getSearchResults()` directly, missing the polling-setup branch. The first `fetch_query_result` poll returned 202, the response handler had no branch for that case, and the spinner stuck. Refresh worked because the component remounts on a hard page load. Fix: pull the "fetch then poll if 202" logic out of `mounted` into `runSearch()`. Both `mounted` and the watch call `runSearch`, so the polling-setup runs in both cases. Also: - `stopPolling()` helper that clears the interval AND nulls `searchPollingInterval`. The previous code only called `clearInterval(...)`; the handle was never reset, so subsequent searches saw a stale truthy value and skipped polling setup. - `runSearch` calls `stopPolling()` and resets `searchTaskId` at the top, so a new query doesn't poll for the previous query's stale task ID. - Drop the dead `await … .then(…)` pattern in `mounted`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Cleanup pass following the ruff/ty/uv migration (#173). Removes config files and accidental commits that no longer serve any purpose, and refreshes the README.
Deleted
.pylintrc,.style.yapf— legacy linter/formatter configs; replaced by ruff in Migrate tooling to ruff, ty, and uv #173.copilot/— AWS Copilot CLI manifest. Deployment is now managed by Pulumi/ECS in theord-infrastructurerepository.dump.rdb— accidentally committed Redis snapshot.Updated
README.md— refreshed: dropped the placeholder "How to Deploy" section, corrected the project layout (the API lives inord_interface/api/, notord_interface/client/; the package is FastAPI, not Flask), marked the editor as legacy, switched setup commands touv sync, and flagged the docker-compose host-port 5432 collision.CONTRIBUTING.md— replacedpython setup.py installwithuv sync..gitignore— added.pytest_cache/,.ruff_cache/,.venv/, anddump.rdbso these stop showing up in future working trees.Out of scope
Editor-tree
# pylint:/# pytype:comments inord_interface/interface.pyandord_interface/editor/py/serve.pyare intentionally left alone — the editor is excluded from ruff/ty and is being removed in #172.Test plan
uv run ruff check ord_interfacepassesuv run ruff format --check ord_interfacepassesuv run ty check ord_interfacepassesgit grepconfirms no remaining references topylint/yapf/copilot/setup.py/format.sh/pytypeoutside the editor tree🤖 Generated with Claude Code