Repository navigation
Sync with streamlit-template main (fixes dropped "false" TOPP parameters) - #103
Merged
Merged
Conversation
…only key names, omitting values.
Mirror FLASHApp's split-build / manifest-merge approach so both linux/amd64 and linux/arm64 are published for the full and simple variants. Existing `<ref>-full` / `<ref>-simple` / `latest` tags become multi-arch manifests — k8s overlays, docker-compose users, and direct `docker pull` callers transparently get the right arch. Dockerfile.arm (delta from Dockerfile): - aarch64 miniforge installer - conditional THIRDPARTY/Linux/aarch64 copy (some OpenMS releases ship an empty/missing aarch64 dir) - pruned thirdparty PATH to tools that actually have ARM builds: LuciPHOr2, MSGFPlus, ThermoRawFileParser, Comet, Percolator, Sage Dockerfile_simple.arm (delta from Dockerfile_simple): - aarch64 miniforge installer only — pyOpenMS ships aarch64 wheels on PyPI, so `pip install -r requirements.txt` works as-is The shared docker/entrypoint.sh is reused as-is on ARM: its apptainer/read-only-root handling is arch-neutral and worth keeping. Base stays ubuntu:22.04 (Redis 6.0 predates the ARM64-COW-BUG warning, so no `--ignore-warnings` flag needed). Workflow changes (build-and-test.yml): - `build` renamed `build-amd64`; per-arch tags carry `-amd64`. - New `build-arm64` job runs on `ubuntu-24.04-arm`, builds the `.arm` Dockerfiles for both variants, ends with a pull-back + /_stcore/health probe on push events. - New `create-manifest` job stitches `<ref>-<variant>-amd64` + `<ref>-<variant>-arm64` into multi-arch `<ref>-<variant>` and `latest` manifests. - test-apptainer / test-nginx / test-traefik / publish-apptainer keep consuming the amd64 artifact only. SIF publishing stays amd64-only this iteration. - PRs build both arches (registry cache keeps warm runs cheap) but don't push; manifest creation also skipped on PRs. Branch-protection note: the `build` required check is renamed to `build-amd64`. Admins should update protected-branch rules and add `build-arm64` / `create-manifest` if those should also be required.
Previously the apptainer/nginx/traefik integration tests only ran
against the amd64 artifact, so the arm64 image was validated solely
by its build succeeding plus a post-push /_stcore/health probe. Now
all three integration matrices fan out over arch=[amd64, arm64] with
a matrix-driven runs-on, exercising the read-only-root apptainer
contract and both kind-based ingress paths on a native ARM runner
too.
Changes:
- `build-amd64` artifact renamed from
`openms-streamlit-<variant>-image` to
`openms-streamlit-<variant>-amd64-image` for symmetry.
- `build-arm64` now also `load: true`'s the built image, retags to
the kind-friendly `openms-streamlit:test`, saves it as a tar, and
uploads it as `openms-streamlit-<variant>-arm64-image`. The
post-push pull-back smoke test is removed — the new apptainer/
nginx/traefik runs subsume it and avoid the slow GHCR pull.
- `test-apptainer`, `test-nginx`, `test-traefik` matrices switched
from `variant: [full, simple]` to an `include:` list with
{variant, arch, runner} tuples; `runs-on: ${{ matrix.runner }}`
selects `ubuntu-latest` for amd64 and `ubuntu-24.04-arm` for arm64.
Artifact download names get `${{ matrix.arch }}` interpolated.
- SIF upload at the tail of `test-apptainer` gated on
`matrix.arch == 'amd64'`: arm64 still runs the full apptainer
contract end-to-end, but only amd64 produces the SIF that
`publish-apptainer` ships to GHCR (HPC SIF consumers are amd64).
Note on `publish-apptainer`: it stays on `needs: test-apptainer`,
which now waits for the arm64 matrix entries too — meaning an arm64
apptainer regression will block amd64 SIF publishing. Conservative
on purpose; happy to decouple via separate jobs if that turns out to
be too strict in practice.
The ARM build of `make -j4 TOPP` failed at the link step with
/usr/bin/ld: /root/miniforge3/lib/libyaml-cpp.so.0.8: undefined
reference to `std::ios_base_library_init()@GLIBCXX_3.4.32'
The conda-forge libyaml-cpp wheel for aarch64 is built against
GLIBCXX_3.4.32 (gcc 13+), but Ubuntu 22.04's system g++ ships with an
older libstdc++. Running cmake inside the mamba shell lets it discover
/root/miniforge3/lib first, so the conda-forge yaml-cpp gets linked
into every TOPP binary and breaks. amd64 happens to work because the
conda-forge amd64 yaml-cpp build is older.
Fix mirrors FLASHApp's Dockerfile.arm: configure OpenMS in two cmake
passes — pass 1 under plain `/bin/bash` with
`-DCMAKE_IGNORE_PREFIX_PATH=/root/miniforge3` so cmake resolves C++
deps from the system tree (libyaml-cpp from contrib, boost from apt,
etc.); pass 2 under `mamba run` with `-DPYOPENMS=ON` so the Python
bindings still find conda-forge Python / Cython / NumPy. The
IGNORE_PREFIX_PATH flag is repeated on pass 2 to keep the cached C++
link command unchanged.
Only Dockerfile.arm changes; Dockerfile (amd64) keeps its single-pass
cmake to avoid disturbing the working x86 path.
The two-pass cmake split from 1d73b67 runs pass 1 under `SHELL ["/bin/bash", "-c"]`, but the only cmake on the image is the one from `mamba install cmake` at /root/miniforge3/envs/streamlit-env/bin/cmake — not on plain bash's PATH. Result: exit 127 (command not found) the moment pass 1 invokes cmake. FLASHApp.arm sidesteps this by installing cmake via apt; do the same here (just append `cmake` to the existing apt-get install line). The mamba cmake install stays, so pass 2 under the mamba shell continues to use the conda-forge cmake exactly as it did before. Ubuntu 22.04 ships cmake 3.22, comfortably above OpenMS 3.5's 3.15 floor.
The previous fix (install cmake via apt) didn't actually help: OpenMS
3.5's CMakeLists.txt requires cmake >= 3.24, and Ubuntu 22.04's apt
cmake is 3.22.1, which fails configure with
CMake Error at src/openms/extern/CMakeLists.txt:11 (cmake_minimum_required):
CMake 3.24 or higher is required. You are running version 3.22.1
That's exactly why the existing x86 Dockerfile installs cmake via
mamba (the conda-forge build is 3.30+). FLASHApp.arm escapes this by
using ubuntu:24.04 (apt cmake 3.28); we stay on 22.04 to minimize
churn vs. the working x86 Dockerfile.
Fix: in pass 1, call the mamba-env cmake by its full path
`/root/miniforge3/envs/streamlit-env/bin/cmake`. The plain-bash SHELL
is still in effect, so cmake doesn't pick up any conda-forge
environment side effects, and CMAKE_IGNORE_PREFIX_PATH keeps it from
auto-discovering miniforge libraries during find_package. The cmake
binary itself runs against miniforge's libstdc++, but that's a
runtime detail of cmake — it doesn't leak into the configured
project's link command.
The apt cmake addition from f11bc99 is now redundant but harmless;
leaving it in place to keep this diff focused.
Two failures in the previous run (test-traefik full, test-nginx simple) ended with the runner reporting "No space left on device" while flushing its diagnostic log. ubuntu-latest starts with ~14 GB free; downloading the full image artifact (5-8 GB), loading it into docker (decompressed, larger), pulling kind's node image, then loading the OCI tar into the kind cluster easily exceeds that budget. Mirror the cleanup already used by `build-arm64`: drop the runner's preinstalled dotnet / android SDK / ghc / hostedtoolcache to recover ~30 GB. Same step now runs at the top of test-apptainer, test-nginx, and test-traefik on both amd64 (ubuntu-latest) and arm64 (ubuntu-24.04-arm) matrix entries — the arm runner is at least as tight as amd64.
After the two-pass cmake configure landed in 5185c3e, the next attempt got past `make -j4 TOPP` (the link error is fixed) but failed fast in `make -j4 pyopenms` with: CMake Error: Not a file: /openms-build/CMakeFiles/VerifyGlobs.cmake CMake Error: Error processing file: /openms-build/CMakeFiles/VerifyGlobs.cmake make: *** [Makefile:11553: cmake_check_build_system] Error 1 `VerifyGlobs.cmake` is generated by cmake for `file(GLOB CONFIGURE_DEPENDS ...)` targets and is consulted by `cmake_check_build_system` at the top of every subsequent `make` invocation. The intermediate cleanup line RUN rm -rf src doc CMakeFiles deleted it, which is fine on the x86 single-pass build (different cmake codepath when PYOPENMS=ON is set in the initial configure, no VerifyGlobs.cmake generated) but breaks the ARM two-pass build. Stop deleting CMakeFiles/ between `make TOPP` and `make pyopenms`. We still drop `src/` and `doc/` for disk savings; keeping CMakeFiles costs only a few hundred MB on the intermediate layer.
eWaterCycle/setup-apptainer@v2 installs apptainer from the upstream .deb asset on the GitHub release. Upstream apptainer only publishes amd64 .debs (verified: every v1.3.x release lists only `apptainer_<ver>_amd64.deb`, no _arm64 / _aarch64 variant). On the ubuntu-24.04-arm runner the action's `apt-get install ./apptainer_*.deb` fails with sudo exit code 100 because the package can't be resolved. Building apptainer from source on the ARM runner would add ~15 minutes and a maintenance surface (Go toolchain, suid configuration) for limited value — HPC SIF consumers remain amd64. Revert test-apptainer to amd64-only and document why. test-nginx and test-traefik still exercise the ARM image via kind, which gives us functional ARM coverage at the docker-runtime level even without apptainer. Side cleanups now that arm64 is gone from this matrix: - artifact name back to a literal `*-amd64-image` (no matrix.arch) - SIF upload gate drops the `matrix.arch == 'amd64'` check
kind/kubectl/helm setup actions fail with "Cache directory '/opt/hostedtoolcache' does not exist". Drop just dotnet/android/ghc (~34 GB) and leave the tool cache in place.
curl exit-22 doesn't tell us whether the pod, service, or ingress is the broken link. Dump pods/logs/ingress/controller logs on failure so the next run surfaces the actual cause.
\`docker load\` + \`kind load docker-image\` keeps the image in both host docker AND each kind node's containerd. With a 5-8 GB image and two kind nodes that's ~25 GB of duplicated storage, which trips the "no space left on device" error in kind's ctr import. Switch to \`kind load image-archive\` so the tar streams directly into each node, and rm the tar after to reclaim /tmp.
503s in test-nginx/test-traefik traced to two issues: 1. The prod overlay maps openms-streamlit -> ghcr.io/openms/streamlit-template:main-full, but the build job was re-tagging the local image as openms-streamlit:test. Rendered manifests pointed at the registry name; kind only had :test loaded; pods stayed ErrImagePull. Retag as :main-full so kind has exactly the ref the manifests use. 2. Three of the four pod specs declare imagePullPolicy: Always; the existing sed only rewrote IfNotPresent. With Always and no registry creds in kind, pods loop on ImagePullBackOff. Extend the sed to catch both.
Add ARM64 (aarch64) multi-architecture Docker build support
…fest create-manifest fails with "ghcr.io/openms/streamlit-template:main-full-amd64 is a manifest list" because docker/build-push-action v5 adds a provenance attestation by default, which buildx packs as a manifest list (image + attestation entries). docker manifest create rejects manifest lists as components. build-arm64 already sets provenance: false for the same reason; mirror that on the amd64 path so both per-arch tags are flat image manifests that can be merged into the multi-arch manifest.
fix(ci): disable provenance on build-amd64 to keep pushes single-mani…
support flag parameters without valuesAllow flags to work with only key names, omitting values.
Add configurable legal links (Impressum, Privacy, Terms)
…iple TOPP tool instances (#354) * Fix tool instance name handling and move custom defaults to params.json Resolves the issue where multiple instances of the same TOPP tool could not have independent custom defaults. The .ini file (keyed by tool name) only supported one set of defaults — the first instance's defaults were baked in and subsequent instances were silently ignored. Changes: - Add get_merged_params() to ParameterManager for centralized three-layer parameter resolution (ini defaults < _defaults < user overrides) - Replace .ini file mutation with _defaults seeding in params.json, keyed by tool instance name - Update save_parameters() to compare against _defaults when determining which values are non-default - Update run_topp() and non_default_params_summary() to use merged params - Resolve instance names to real tool names across save_parameters, input_TOPP, run_topp, and get_topp_parameters * Remove redundant -ini flag from run_topp The .ini file is no longer mutated with custom defaults — it stays pristine. All parameters (_defaults + user overrides) are already passed as CLI flags via get_merged_params(), making -ini redundant. * dont disable cross origin --------- Co-authored-by: Tom David Mueller <tom.mueller@beachouse.de> Co-authored-by: Tom David Müller <57191390+t0mdavid-m@users.noreply.github.com>
…l UI (#398) * InputTOPP reactive parameter * docs: document reactive/conditional-UI pattern for workflow widgets Add developer + agent-facing documentation for the reactive=True option on input_widget, select_input_file and input_TOPP, which un-isolates a parameter widget from its st.fragment so a change reruns configure() and enables conditional UI. - docs/toppframework.py: "Reactive parameters (conditional UI)" block with a combined input_widget + input_TOPP example in the Parameter Input section - .claude/skills/create-workflow.md: UI-widget note, Conditional UI section, checklist item - CLAUDE.md: reactive note in the TOPP Workflows section - docs/build_app.md: pointer to the framework doc --------- Co-authored-by: jkvision1101 <jkvision1101@fullseeomics.com> Co-authored-by: Tom David Mueller <tom.mueller@beachouse.de>
* Add support for boolean CLI flag parameters in TOPP tools Allows input_TOPP() to designate parameters as flags (present/absent) instead of key-value pairs, persisted to params.json and session_state so run_topp() can correctly build the command line. * Add unit tests for TOPP flag parameter support Exercise run_topp()'s flag-aware command construction and the flag-source contract written by input_TOPP(). Working-behaviour tests cover: bare flag emission for truthy bool/"true" values, omission for falsy/"false", empty and None regular params skipped, 0/0.0 preserved, multiline splitting, custom_params scalar/list/flag handling, and params.json-vs-session_state precedence. A TestKnownCodeRabbitBugs group asserts the correct behaviour for the two CodeRabbit findings (per-tool flag fallback; list expansion / empty-list skipping). Those tests fail against the current code, documenting the defects until they are fixed. * ci: run continuous-integration on pull_request The continuous-integration workflow (which runs `pytest test_gui.py tests/`) was triggered on push only, so pull requests from forks never executed the tests/ suite as a check. Add the pull_request trigger so the test job gates PRs as well. * Fix flag-parameter command construction and green the test suite Address both CodeRabbit findings on run_topp() (CommandExecutor.py): - Per-tool flag fallback: consult session_state per params_key instead of only when the whole _flag_params map is empty, so one tool's saved flags no longer shadow another tool's session flags. - List handling in the non-flag branches: skip empty lists in both the merged and custom loops, and expand non-empty merged lists into separate args instead of stringifying them as "['a', 'b']". Move the four tests that pinned these findings out of TestKnownCodeRabbitBugs into the behaviour classes they now guard (they pass with the fixes applied). Also fix a pre-existing, unrelated CI failure in test_queue_manager_cancel.py: test_stopped_status_is_mapped_in_get_job_info compared JobStatus enum members by identity via a re-import, which mismatches once other test modules pop src.workflow.* from sys.modules. Compare .name instead — reload-proof and semantically equivalent. --------- Co-authored-by: Tom David Mueller <tom.mueller@beachouse.de>
Both full-variant image builds have failed since 2026-08-25: AttributeError: module 'Cython.Compiler.Nodes' has no attribute 'CConstTypeNode'. Did you mean: 'CBaseTypeNode'? ... buildx failed: process "mamba run -n streamlit-env /bin/bash -c make -j4 pyopenms" did not complete successfully: exit code: 2 Line 65 of both Dockerfiles pins autowrap but not cython. cython 3.3.0 was released on 2026-08-22 and removed Cython.Compiler.Nodes.CConstTypeNode and CConstOrVolatileTypeNode, which autowrap 0.24's PXDParser._check_type_constness calls while generating the pyOpenMS bindings. The build resolved cython-3.2.9 on 2026-08-21 and succeeded; it resolved cython-3.3.0 on 2026-08-25 and did not. 3.2.9 is the last 3.2.x on PyPI, so `<3.3` restores exactly the resolution that was green. The lower bound matches what pyOpenMS asserts for itself during the build (`Cython>=3.1.0`). Bumping autowrap instead does not work: the newest release is 0.27.0, uploaded 2026-01-14, seven months before cython 3.3.0, and it declares only `Cython >= 3.0` - an open upper bound with no knowledge of the removed AST nodes. No published autowrap can be assumed to handle them, and a build that happened to succeed would silently change the generated bindings. Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add failing tests for multi-node distribution
Tests first, committed red. Each one fails against the current code for a
reason worth naming:
- test_parameter_integrity: a failed params.json read currently returns {},
which save_parameters merges and writes back, erasing _defaults,
_flag_params and every other tool's values while telling the user it was a
deliberate reset.
- test_tasks: tasks.py logs WORKFLOW FINISHED unconditionally and returns a
dict from its exception handler, so RQ records every job successful and
FailedJobRegistry is structurally empty. This module had no coverage at all.
- test_seed_demos: the seed-demos initContainer uses `cp -rn`, which under
replicas: 2 either skips a file another pod is still writing, permanently,
or exits 1 on EEXIST into Init:CrashLoopBackOff.
- test_storage_health / test_sidebar_monitors: for a storage indicator that
must read Redis only. A stat on a hard NFS mount blocks in uninterruptible
sleep, so a widget that touches the filesystem becomes the hang it reports.
docker/seed-demos.sh is extracted so the manifest and the test share one copy
rather than drifting. .gitattributes pins *.sh to LF: a CRLF checkout breaks
both sh in the container and the test.
test_topp_flag_parameters keeps its argv assertions unchanged; only the
fixture is hardened, because thread resolution is about to depend on a
settings.json read relative to the working directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Make workflow failures observable
Nothing downstream can be validated while every failure reports as success,
so this lands first.
The success signal was inverted in both directions at once. Locally,
Workflow.execution() was annotated -> None and returned nothing, so the
WORKFLOW FINISHED marker was never logged and a successful run rendered
"Errors occurred". In queue mode tasks.py logged that marker unconditionally
and returned a dict from its exception handler, so RQ recorded every crash as
a success and health.py's failed_jobs was structurally always zero. Both are
fixed together: honouring the bool while execution() still returned None
would have made every workflow report failed.
execution() now also gates on each run_topp/run_python result. Those already
returned False on a non-zero exit and nobody looked, which is why a tool that
died mid-run still reported success. A subclass still declaring -> None keeps
working, with a deprecation warning in the workflow log.
params.json reads are split rather than flagged. get_parameters_from_json
stays tolerant for the two constructors and five read-only callers, but no
longer launders a read failure into {} silently, and its st.error is now
conditional on a real session — tasks.py builds a ParameterManager in the RQ
worker where it was a silent no-op. The read-modify-write-back sites use a
strict read that raises and aborts the write instead of persisting {}.
StreamlitUI's input_TOPP path gets more than a raise: it runs on every
configure() render, so it preserves the file and offers a reset rather than
dumping a traceback on the parameter page.
max_threads.online has been dead code since PR #333: the worker has no
ScriptRunContext, so st.session_state.get("settings", {}) is {}, the online
branch was unreachable, and it silently used the hardcoded local default of 4.
A Streamlit-free settings loader fixes that, and the result is memoised
because run_topp resolves it twice and both reads must agree.
create-workflow.md documented the broken -> None signature, which would have
regenerated the bug in every new workflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Tolerate a shared, remote, restartable filesystem
Every change here is a no-op or an improvement on the current single-node
volume, so it ships before the storage cutover rather than with it. That way
the cutover has working instrumentation to be watched with.
The seed-demos initContainer no longer uses `cp -rn`. It copies to a private
temp directory and renames: rename(2) on a directory is atomic, so the winner
renames and the loser cleans up and exits 0. A lock file would be the wrong
tool. It also gets a bounded timeout and exits 0 on failure, because it blocks
pod start on the mount and would otherwise defeat the decision that the UI
survives a storage outage.
The storage indicator reads Redis and never the filesystem. A worker with the
mount refreshes a per-node key with a TTL, so the TTL is the liveness
mechanism and there is no timeout logic to get wrong. Per-node keying matters
because a single healthy node would otherwise mask a wedged one. It reports
three states, not two: connected, unreachable, and unknown when Redis itself
is down — a red indicator caused by a dead Redis sends an operator debugging
the wrong layer.
monitor_hardware is now hidden in online mode. It reads psutil on the
Streamlit pod, which does no work once execution is distributed, and would
show an idle web pod while a worker saturates.
The cleanup CronJob gets activeDeadlineSeconds. It is concurrencyPolicy:
Forbid, so one run hung on a storage restart would block every subsequent
night, silently and permanently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Serve the workspace volume over NFS
The app could not use more than one node: a ReadWriteOnce Cinder volume
attaches to exactly one, and four workloads mount it. This runs NFS-Ganesha
over that volume and hands out a ReadWriteMany claim instead.
k8s/storage/ is a separate kustomize root rather than a component, because
k8s/base sets `namespace: openms` and the namespace transformer runs after
patches — it would clobber any per-object namespace, and components inherit
the parent's transformers. It is deliberately prefix-free so namePrefix never
has to be duplicated into a Helm values file.
workspaces-nfs-pvc is a NEW claim, not an edit of workspaces-pvc. A bound
PVC's spec is immutable apart from resources.requests, so apply would be
rejected outright, and the only way past that is deleting a claim whose
cinder-csi class reclaims with Delete — destroying the volume the rollback
depends on. The mountPath is unchanged, and every consumer already reaches the
volume through claimName plus /workspaces-streamlit-template, so nothing in
src/ changes and absolute symlinks and external_files.txt survive untouched.
Four Ganesha settings are load-bearing and commented as non-tunable.
existingClaim and reclaimPolicy: Retain keep the data across pod and PV
lifecycles. strategy: Recreate avoids the deadlock where a new pod cannot
mount the claim the old one still holds. device-based-fsids: false is the
subtle one: the default derives NFS file handles from the backing device's
major/minor, and a Cinder /dev/vdX minor is not stable across re-attach, so
every client would ESTALE after the first restart — weeks later, with nothing
to link it to the cause. An explicit memory limit is set because N concurrent
write streams put N sets of buffers in one userspace process, and OOM there is
the one genuine collapse mode in this design.
The NetworkPolicy needs two allow rules, not one. Ganesha exports
no_root_squash, and the namespace is shared with another application, so a
pod-label rule alone is the isolation that matters — but the provisioner emits
in-tree nfs: PVs which the kubelet mounts from the node's own address in the
host network namespace, matching no podSelector. Without the second rule every
mount hangs. It ships as a placeholder that must be narrowed to the real node
addresses.
CI applied only the prod overlay, so a storage root it never applied would
have stranded every pod; it now applies both. The `|| true` on the deployment
availability wait is removed — a Pending, CrashLooping or unschedulable
Deployment passed silently, leaving the ingress curl as the only real gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Let the scheduler place worker pods
Removes per-node memory tiering entirely. Nothing in k8s/ now pins a pod to a
node — no nodeSelector, no nodeName, no nodeAffinity, no openms.de/memory-tier
labels. A tier survives only as a pod size, which is a request the scheduler
can act on rather than a label an operator has to maintain.
The memory-tier components were already the right shape; they just carried a
nodeselector.yaml patch targeting `kind: Deployment` with no name, which
landed on redis and streamlit as well as the worker while missing the CronJob
entirely. Deleting the patch files and their kustomization entries is atomic:
a kustomization referencing a deleted path fails to build.
requests now equal limits in both tiers. That is architectural rather than
tidy: the pod size is the tier, so a worker requesting 1Gi against a 16Gi
limit would be schedulable anywhere and then OOM. Guaranteed QoS also moves
oom_score_adj from roughly 937 to -997, so the worker stops being near the top
of the node's kill list.
rq-worker moves to a fixed replica count with topologySpreadConstraints over
kubernetes.io/hostname. Those constraints were inert before: maxSkew is
measured over eligible domains, and the nodeSelector left exactly one node
eligible, so any spread rule written earlier would have passed while changing
nothing.
k8s/overlays/ci exists because a production-sized worker cannot be scheduled
on a CI runner; it is prod with the worker shrunk, so the kind jobs exercise
the real manifests rather than a hand-maintained copy.
docs/kubernetes-deployment.md claimed the VolumeBinding plugin pins pods to
the node holding the attached volume. It does not — the scheduler checks PV
node affinity only, and the result is a Multi-Attach hang at attach time
rather than a scheduling refusal. That belief is why the old topology looked
intentional.
configure-k8s-deployment.md had no question about worker size or replica count
at all, and told operators co-location was enforced by the RWO mount. Both are
now wrong and both are fixed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Pin the CI toolchain and make its gates fail honestly
Every job that renders k8s/storage/ was failing on one thing: azure/setup-helm
was unpinned at four sites and `latest` now resolves to Helm v4.2.4, which
removed the `-c` shorthand that the kustomize inside kubectl uses to probe it.
`kubectl kustomize --enable-helm` refused outright, so assert-invariants died
in its first assertion and all eight kind jobs died before applying a single
manifest - with none of the eleven new assertions ever executing.
Pin helm to v3.21.4 at all four sites, and pin kubectl and kind-action too:
setup-kubectl resolved `latest` from the frozen legacy GCS bucket (v1.31.0,
kustomize 5.4.2) while the kind jobs ran v1.35.0 / 5.7.1, so the static gates
validated output the clusters never saw. Not fixed by bumping kubectl past
1.36 instead: kustomize 5.8.1 stops applying the namespace transformer to
Helm-inflated objects, which would deploy Ganesha outside its NetworkPolicy
for anyone using the documented apply pipe.
Unmasked by that, four gates that could not fail:
- lint-manifests piped a failing kustomize into kubeconform under a shell with
no pipefail, printed `Valid: 0`, and passed in 11s. The chart it exists to
validate had never been validated. Render to a file and require it non-empty
- kubeconform exits 0 on empty input, so pipefail alone would not catch a
render that succeeds and emits nothing.
- The storage apply loop used `if ...; then break; fi`, which never trips
`set -e`, so total failure fell through to a command substitution that killed
the step before the authored error message could run.
- The Traefik host list is derived through `grep -oP | tr`; a miss yields an
empty string with exit 0, and the only curl of the app through Traefik then
iterated zero times, green. Also `--set service.type=ClusterIP` is a dead
values path in that chart - the key is service.spec.type - so the Service had
been LoadBalancer all along.
- assert_workers_spread_across_nodes walked only the selector keys that were
present, so a constraint that lost its app scoping matched the pod template
and passed. That is precisely the regression the base manifest's own comment
calls load-bearing.
Adds two assertions. assert_netpol_admits_every_node checks from the cluster
that every node InternalIP is admitted on 2049, because the node CIDR is
rewritten by CI rather than shipped and a sed whose target moves fails silently
forty minutes later as an unrelated timeout. assert_storage_identity_values
covers the three quarters of invariant 2 that nothing checked - Retain, the
fixed backing claim, the single replica - each of which renders, deploys and
passes every runtime assertion while costing deleted data.
Also rewrites the kind node CIDR (kindnetd enforces NetworkPolicy, and the
kubelet mounts in-tree nfs: PVs from the node address, so the shipped RFC 5737
placeholder hangs every cross-node mount), sizes the CI claims to what the
runner can actually provide, and drops the `-l app=${SLUG}` selectors from the
failure dump - SLUG is exported after the storage steps, so every job that died
early dumped an empty namespace and looked like an empty cluster.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Size the worker from its cgroup, and harden two RWX-sensitive spots
Three corrections that the move to a genuinely shared volume makes matter,
kept deliberately narrow - the rest of the review backlog is a follow-up.
max_threads now comes from the container's own cgroup CPU quota in online
mode, with max_threads.online left as the fallback for deployments that
expose none. Reading settings.json instead of session state fixed the worker
reading a hardcoded 4, but replaced it with a single static number that
cannot describe both a 4 cpu low-tier worker and a 20 cpu high-tier one - and
described neither. The worker runs at requests.cpu == limits.cpu, so its pod
spec is its allocation. Both cgroup generations are handled, quotas round up
(1500m is two useful threads, not one), and "no quota" reports None rather
than 1 so a laptop does not silently serialise.
The trade-off is deliberate and user-visible: an operator can no longer
under-subscribe a container that declares more CPU than it wants used.
_write_parameter_file keyed its scratch file on os.getpid() alone. Streamlit
serves every session from threads of one process, and that process is pid 1 in
every replica, so two sessions sharing a ?workspace= link - or two pods, now
that the volume really is ReadWriteMany - resolved to the same path and
interleaved into it, publishing exactly the torn file the function exists to
prevent. Adds a uuid, plus flush+fsync before the rename, since os.replace
orders the directory entry and not the data behind it.
allowVolumeExpansion was the one storageClass key still inherited in a values
file that restates its defaults on purpose. The chart defaults it true and an
in-tree nfs: PV has no expander, so a resize is accepted and then sits in
FileSystemResizePending forever.
The existing max_threads tests pin detect_cpu_quota to None explicitly rather
than trusting the runner to be unconstrained, and reach it through the
function's own globals - the module-globals idiom this file already uses,
because src.workflow.* is popped from sys.modules and a dotted-name patch
would target a second copy of the module.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Release the NFS mounts before kind tears its nodes down
All eight kind jobs are red, and every one of them is red only in the post
step. Their 25 normal steps all concluded success - including every runtime
assertion, up to and including "4 pods mount template-app-workspaces-nfs-pvc
across 2 nodes", which is the acceptance criterion for this whole PR.
`docker rm -f` on a kind node whose kubelet still holds a `hard` NFSv4.1 mount
cannot complete. The Ganesha server is a pod inside the cluster being destroyed,
so it dies alongside its own clients; the pending RPCs block in uninterruptible
sleep, SIGKILL is ignored, containerd never sees an exit event, and dockerd
gives up after ~12s.
This is not a regression. Round 1 teared down cleanly because it died before a
single NFS mount existed - the wedge is the first evidence the mounts are real,
and it is a live demonstration of the D-state mechanism the rq-worker probe
comments reason about. Production never destroys an NFS server and its clients
in one `docker rm -f`, so this is a CI-harness gap rather than a product defect.
Release the mounts in a last normal step. Normal steps all run before any post
step, which is also why "Dump cluster state on failure" is `skipped` in these
jobs - no `if:` on a normal step can observe a post step failing, so the only
red jobs in the pipeline currently ship zero diagnostics.
The order cannot be rearranged: diagnostics first, because after the deletes
there is nothing left to name; then the clients, while the server is still up
and their unmounts can complete; then a lazy detach for whatever the kubelet
did not release, which returns without waiting on the server and so cannot
itself hang; then the server, because killing it while a client mount is still
attached is the deadlock verbatim.
Deliberately NOT `ignore_failed_clean: true` on kind-action. That input is real
and would green all eight jobs immediately, but it works by discarding the
error - permanently deleting the only signal that teardown wedged, including if
this step later regresses through a cluster-name typo. The residual-mount echo
at the end is the check that replaces it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Close three silent-failure paths in the kind jobs, and pin ingress-nginx
Four small fixes to the CI steps, none of which fired in the last run - which
is the reason to make them now rather than while debugging something else.
The two app-overlay apply loops still had the shape the previous commit claimed
to have fixed. It fixed the two STORAGE apply loops and left these: `kubectl
apply` inside an `if` condition is exempt from `set -e`, so after five failures
the last command run is `sleep`, the loop exits 0, and the step passes having
created nothing. The symptom would surface three steps later as "Wait for Redis
to be ready" timing out on a pod that never existed. All four loops now track
an `applied` flag and exit explicitly.
ingress-nginx was the last unpinned upstream of the class that took this whole
pipeline down through Helm 4, and here the usual moving-ref trap has inverted
into rot: the repository is archived, so `main` will not move again, but the
kind node image will. Pinned to controller-v1.15.1, whose support table lists
k8s 1.35 - not v1.14.0, which stops at 1.34 while these jobs create
kindest/node:v1.35.0.
The Traefik host discovery read `.spec.routes[0].match` only. Prod renders a
single route carrying both hostnames so this changes nothing today, but a
second route would have dropped silently out of the only gate that exercises
the app through Traefik.
The NFS module probe reported "absent" about a working host. `grep nfs
/proc/filesystems` printed nothing on every node of every job in a run where
those same nodes went on to mount type=nfs4 vers=4.1 two minutes later - the
module autoloads on first mount, and this step runs before any mount exists.
It now attempts a modprobe and, if the grep still finds nothing, says why the
answer is not conclusive. This is the line a triager reads to decide whether a
hung mount is a policy problem or a missing module, so it needs to not lie.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Stop three assertions passing on conditions they exist to catch
All thirteen assertions ran green last round. Two of them could not have gone
red for the thing they claim to check, and a third never looked at the object
whose placement mattered most.
assert_netpol_admits_every_node never checked the port. It collected
`.from[].ipBlock.cidr` without descending into the sibling `.ports`, and
flattened across every policy in the namespace, while its PASS line asserted
"admitted on 2049". Accidentally correct on today's single-rule policy; change
the port to 111, or add any unrelated policy carrying a broad ipBlock, and it
still reported success while every cross-node mount hung - the forty-minute
Deployment-availability timeout its own docstring exists to pre-empt. Rules are
now filtered by port before their CIDRs are collected, keeping a ports-less
rule as all-ports. Verified against synthetic payloads: the happy path is
unchanged, port 111 now yields empty and trips the guard, an unrelated policy
on 9100 is excluded, and UDP/2049 is excluded.
assert_storage_identity_values tested the backing claim with `-z` where its two
siblings use equality, so `persistence.existingClaim` and the PVC name in
nfs-backing-pvc.yaml - hand-synced, as the values file itself concedes - were
verified by nothing. Rename the PVC without touching the values and the server
would sit Pending on a claim that does not exist, with all four static
assertions green. Now compared against the rendered PVC name, in an `else` so a
dropped claim reports once instead of twice. Both workload selects are also
scoped to the server's own app label, derived from `nameOverride` rather than
hardcoded, so a chart that grows a second workload produces a real answer
instead of two lines and a message naming neither.
assert_no_node_pinning_anywhere never saw the Ganesha StatefulSet. Its rendered
half rendered only the prod overlay, and its text half asks git, which prunes
the vendored chart under k8s/storage/charts/ through .gitignore - so the pod
this entire PR is about placing was checked by neither half, and neither was
k8s/overlays/ci/, which is what the kind jobs actually apply. It now scans
three rendered roots through a shared helper. The PASS string is rewritten
accordingly: it previously ended by stating that the vendored chart is not
scanned, which would have become false.
PersistentVolume is skipped by that scan. `spec.nodeAffinity` on a PV is volume
topology, not pod pinning, and is mandatory for a local volume. Nothing in the
three roots renders a PV today, but the storage root is now among them and a
chart that ever emitted one would have turned invariant 1 permanently red with
no way to green it except weakening the check.
The scan requires helm outright rather than `if command -v helm`. A check that
silently does less work when a tool is missing is the shape this round exists
to remove.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Ship the storage runbook, decisions and defect register
Eleven tracked files cited A16-RUNBOOK.md, A16-DECISIONS.md or DEFECTS.md as
the authority for why a value is what it is - and none of the three was in the
repository. They sat in an untracked scratch directory that only ever existed
on the machine the multi-node work was done on.
That includes the only documented rollback for the storage cutover. As it
stood, merging would have shipped a storage tier whose escape hatch pointed at
a file nobody else has, with k8s/base/workspace-pvc.yaml telling the next
operator to consult section 5 of something they cannot open.
All three are now under docs/, with every reference repointed:
A16-RUNBOOK.md -> docs/a16-storage-runbook.md
A16-DECISIONS.md -> docs/a16-storage-decisions.md
DEFECTS.md -> docs/storage-defect-register.md
Each gains a header saying what it is and how far to trust it. They are records
of the multi-node work rather than living documents: cluster-specific values in
the runbook are examples, the decision record is a snapshot whose reasoning the
code supersedes where they disagree, and the defect register lists what was
found rather than what was fixed - several entries are still live, which is
worth stating at the top rather than leaving a reader to infer.
content/documentation.py opens named files rather than globbing docs/, so these
three add no chapter to the in-app documentation page and
test_gui.py::test_documentation is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFgL1SSeMCM3J1ZuAVTXv7
* Point the storage docs at each other by the names they now have
51a2b6c moved the three research documents under docs/ and repointed every
reference from k8s/, .github/, src/ and tests/. It missed the eight references
the documents make to each other, which kept their pre-move names: A16-RUNBOOK.md,
A16-DECISIONS.md and DEFECTS.md, none of which exists at any path in the tree.
One of those was the rollback procedure for the storage cutover. k8s/base/
workspace-pvc.yaml cites docs/a16-storage-runbook.md as authoritative for it, so
the outer hop resolved and the inner one led nowhere - the same dead end R4 was
written to close, one level further down.
Also replaces the hardcoded node name in the runbook's pre-flight check. That
command decides whether clients can mount via the in-tree nfs: type or whether
the cluster needs csi-driver-nfs, and it named one Kubermatic-generated node
that has almost certainly been rotated away since. The line immediately below it
already says to run it against both nodes, so it now shows how to list them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8
* Stop four assertions claiming more than they check
assert_netpol_string_matches_overlay failed only when the ipBlock set was EMPTY,
so the most dangerous possible value passed: 0.0.0.0/0 kept it green AND removed
the TEST-NET-1 placeholder NOTE, which matches only ^192\.0\.2\. - the worst
setting produced a quieter log than the safe one, over a no_root_squash export.
It now hard-fails on 0.0.0.0/0 and on any prefix shorter than /12, and its PASS
names the node rule instead of claiming the policy admits exactly one app.
A prefix threshold rather than a "warn on non-RFC1918" rule, which inverts the
hazard: pod networks ARE RFC1918 - kind defaults to 10.244.0.0/16 - so that rule
would stay silent on 10.0.0.0/8 while firing on the harmless placeholder. It
bounds width, not overlap; deciding overlap needs the cluster's own pod CIDR.
assert_no_node_pinning_anywhere skipped an empty CI_ASSERT_* root with a bare
`continue` while its PASS went on naming all three, degrading the sentence to
"prod, and k8s/storage" with a double space as the only trace. That is the
silent-skip shape the hard `_ci_require kubectl helm` eight lines above was added
to remove. Empty is now a failure, the PASS is built from the roots actually
scanned, and each root logs its document count - the step previously emitted its
PASS sentence and nothing else.
The pinning text scan piped through `grep -v nodeSelectorTerms`, which looked
like it suppressed a false positive and did not: `nodeSelectorTerms:` on its own
line does not match _ci_pin_re at all, because the pattern requires nodeSelector
to be followed by `:`, end-of-line or `/`. Its only effect was dropping lines
that DID match and contained the substring - a flow-style nodeAffinity, which is
genuine pinning. Verified a no-op against the current tree.
assert_storage_identity_values derives the server's app label from nameOverride
but never named it in the PASS, so a green log did not record which workload was
inspected. And assert_netpol_admits_every_node discarded jq's stderr, so a syntax
error in its filter surfaced as "the NetworkPolicies admit no ipBlock" - pointing
at the manifests when the fault is in this file. Neither bash -n nor shellcheck
parses an embedded jq program.
Adds assert_doc_links_resolve: every backticked *.md path in tracked Markdown has
to name a tracked file. Deliberately general rather than a grep for the three
filenames the previous commit fixed - that grep would itself be a hand-synced
literal, correct until the fourth rename and silent afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8
* Make four CI steps say what they actually did
The teardown step added last round released NFS mounts before kind destroys its
nodes, but every command in it is `|| true` and it discarded the one diagnostic
that would show whether it did anything. A green teardown was therefore
ambiguous between "the mounts were released" and "there were none to release" -
and if `kind get nodes` had returned nothing, all three loops would have iterated
zero times and the step would have exited 0 having done nothing at all.
It now derives the node list once and reuses it (the cluster can disappear
between loops during teardown), warns when that list is empty, counts the NFS
mounts present before teardown and warns when there are none, and warns when any
survive. It stays non-gating: it runs `if: always()`, including on jobs that died
before creating a cluster, so a gating version would manufacture red on unrelated
failures. Exercised against all four outcomes with stubbed kind/docker/kubectl.
Also drops the `2>/dev/null` from the D-state probe in the same step. Suppressed,
a failing ps prints nothing - byte-identical to "no processes in D state", a
false negative in the diagnostic that identified the deadlock in the first place.
ps stays rather than /proc/<pid>/status because wchan is the column that named it
(nfs_wait_bit_killable); kind v0.31.0's base image installs procps.
The NFS module probe threw away modprobe's exit status, so a SUCCESSFUL modprobe
followed by no /proc/filesystems match - conclusive evidence that a node cannot
mount NFS - printed the same "only conclusive once a mount has been attempted"
hedge as a modprobe that could not run. It now distinguishes them.
shellcheck was behind `if command -v`, so a missing shellcheck produced a green
step with no line saying so; since shellcheck prints nothing when it finds
nothing, the log could not tell "ran clean" from "never ran". Now unconditional,
matching the hard `_ci_require kubectl helm` the same round introduced.
/tmp/prod-rendered.yaml had no emptiness fence, unlike its two siblings. `bash -e`
catches a render that fails, not one that succeeds emitting zero documents - and
that case leaves PROD_RES empty while CI_RES holds a real value, so the
ci-vs-prod worker comparison is FALSE, passes, and prints a real worker size
having compared it against nothing.
Finally, the ingress-nginx pin comment claimed the cluster's k8s version "floats
upward" because node_image is unpinned. It does not: `version: v0.31.0` fixes the
default node image at kindest/node:v1.35.0, and the same comment states that
value three lines earlier. Reworded, with the verified support range.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8
* Give each consumer a build dependency at the granularity it needs
`needs:` in GitHub Actions is job-level, not matrix-leaf-level. With one
`build-amd64` job carrying a {full, simple} matrix, a failure in EITHER leg
marked the whole job failed and skipped every consumer - including test-nginx and
test-traefik, which are the only place the multi-node storage tier is exercised
on a cluster at all.
On 2026-08-25 an upstream cython 3.3.0 release removed CConstTypeNode, which
autowrap 0.24 calls, and broke the `full` build on both arches. All 8 kind leaves
were skipped without their matrices ever expanding, even though both `simple`
images had been built, uploaded and were sitting unexpired in the same run. A
break in one image variant cost 100% of the storage-tier runtime coverage instead
of 50%, and R1, R2, R3, R7, R8 and R9 all went from failing to untested.
build-amd64 and build-arm64 become four jobs - build-{full,simple}-{amd64,arm64} -
routed through a new reusable workflow so that is not four copies of the same
eighty lines. The two jobs were already near duplicates of each other; the only
behavioural difference between them was which ran the disk-space step, and that
is preserved exactly through a `free_disk_space` input rather than quietly
normalised, so the split introduces no delta to compare against when the kind
suite finally runs.
The kind jobs keep `needs:` on all four builds - that is what provides ordering -
and add `if: always()` to override only the other thing `needs:` does, which is
skip on a failed dependency. The condition is the `simple` pair specifically:
those legs install a prebuilt pyopenms wheel and are the robust ones, while the
`full` legs compile OpenMS from source and are where upstream breakage lands.
When `full` breaks, the simple leaves now run and assert everything and the full
leaves go red at the artifact download - honest, since their image genuinely does
not exist, and a new step names the cause rather than leaving "unable to find any
artifacts". When the simple builds are themselves broken there is nothing to test
against and the whole job skips as before. No staleness risk:
download-artifact@v4 is scoped to the current run.
create-manifest now also depends on both kind jobs. It publishes `latest` and the
multi-arch tags the cluster pulls, and nothing in the graph named test-nginx or
test-traefik - so merging published an image whose storage tier had never been
exercised on a cluster. `main` carries no branch protection, so the graph is the
only place this can be enforced. Accepted cost: every push to main now waits on a
~40 minute kind matrix.
test-apptainer is deliberately NOT given the same override. Apptainer is a side
deployment - the cluster runs the docker image - so the coverage worth rescuing
from a one-variant break is the kind suite, not that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8
* Put the missing space back into the NetworkPolicy PASS
The reworded PASS from the previous commit interpolated the admitted CIDRs with
`tr '\n' ' '` and relied on a trailing space that is never there: command
substitution strips the trailing newline, so a single CIDR produced
"plus ipBlock 192.0.2.0/24for kubelet mounts".
Cosmetic, but it is a message about what the check verified, in the round whose
whole subject is messages claiming exactly what their code did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018tuhTmRVKxCXo5gJSJU8e8
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workflow): stop losing widget input on Streamlit < 1.50
`_input_widget_impl` fed the persisted parameter back into every widget as
its initial-value argument (`value=` / `default=` / `index=`). On Streamlit
< 1.50 that argument is hashed into the widget's element id, so the id
changed on every interaction; the following interaction then arrived under
the now-stale id and was silently discarded.
The visible effect was in the mzML file selector: selecting six files kept
only three, because every second click was dropped.
Verified against the real `StreamlitUI._input_widget_impl` and
`ParameterManager` on streamlit 1.49.1, the pinned version:
before: 6 clicked -> 3 stick ['A.mzML', 'C.mzML', 'E.mzML']
after: 6 clicked -> 6 stick
Streamlit ignores the initial-value argument once a keyed widget already
has an entry in session state, so seeding on first render only is
behaviour-preserving: `apply_preset()` and `clear_parameter_session_state()`
already delete the session keys when they want params.json to take effect
again. Regression-tested for text, number (int and float), checkbox,
selectbox, slider and multiselect - first-render seeding, reload from
params.json and value type are all unchanged, on both 1.49.1 and 1.53.1.
Also sorts the `select_input_file` options: `path.iterdir()` is unordered,
so both the displayed order and (on < 1.50) the element id varied between
restarts.
Streamlit >= 1.50 stopped hashing `default` into the element id and was
already unaffected; this fix is version-independent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
* fix(ci): bump pinned cmake to 3.31.12, the pin no longer resolves
`build-openms` started failing on every branch with:
cmake - cmake not installed. The package was not found with the source(s) listed.
Version was specified as '3.31.1'.
Chocolatey delisted that exact patch: its package listing feed now returns
3.31.6, 3.31.10, 3.31.11 and 3.31.12 for the 3.31 series, but not 3.31.1.
Nothing in this repo changed - the last green run was 2026-08-29.
Moves to 3.31.12, the newest patch in the same minor series, deliberately
staying off cmake 4.x: 4.0 dropped compatibility with
`cmake_minimum_required(VERSION < 3.5)`, which the OpenMS build still relies on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
* fix(tests): make the streamlit mock in test_tool_instance_name actually bind
The module swapped `sys.modules['streamlit']` for a MagicMock, imported
ParameterManager, restored the real streamlit, and only then dropped the cached
`src.workflow` modules. That ordering is wrong: a module binds `st` once, at
import time, so if any earlier-collected test had already imported
`src.workflow.ParameterManager` against the real streamlit, the import here was
just a cache hit and the mock never took effect.
The result was a test that passed alone and failed in a full run - the six
`TestSaveParametersWithInstanceName` cases asserted against
`mock_streamlit.session_state` while ParameterManager was reading the real one.
Drops the cached modules *before* the import so the mock binds, and again
afterwards so later test files re-import against the real streamlit.
pytest tests/test_results_helpers.py tests/test_tool_instance_name.py
before: 6 failed, 6 passed after: 12 passed
pytest tests/test_workflow_manager_stop.py tests/test_tool_instance_name.py
before: 6 failed, 5 passed after: 11 passed
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add the notebook-to-webapp skill framework
Six skills that turn a Jupyter notebook into an app built on this
template, plus the docs and glossary they refer to.
`notebook-to-webapp` orchestrates; the five stage skills also work
standalone against an existing app:
capture-notebook-workflow classify cells, extract a Streamlit-free
python tool, golden-value test
interview-parameters one batch review of demo shortcuts and
config parameters, ranked by provenance
scaffold-workflow-app WorkflowManager subclass, four pages,
registration, template cleanup, smoke run
build-insight-dashboard link graph, wireframe, panels, style
verify-webapp-usability headless-browser gate for pages AppTest
cannot see
Users enter through the Quickstart page, which holds a prompt they paste
into an agentic terminal; it names the orchestrator by file path, because
a repo cloned mid-session has its skills on disk but not in the running
session's registry.
The eight existing task playbooks move from `<name>.md` to
`<name>/SKILL.md` with frontmatter, so they surface as loadable skills
alongside the new ones rather than needing to be found by hand.
Documentation gains a "From Notebook to Web App" chapter, rendered from
docs/notebook_to_webapp.md like the others. Design rationale is in
docs/notebook-to-webapp-design.md and docs/adr/0001..0005; the glossary
is CONTEXT.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The template's own source is machinery in a user-facing turn
A build explained a preset that would not refresh by naming
`StreamlitUI.preset_buttons`, `ParameterManager.apply_preset:661` and "a
Python tool" to a mass spectrometrist. Its decision was right — it moved
the button into the workflow rather than patching shared template code —
but the sentence handed over two class names and a line number.
The rule against slipping into instrument language when something goes
wrong already covered this in spirit; every example it named was one of
the framework's own checks, so the template's source read as outside it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The ending never asks an open question
A build closed with "The Results page reads well as it stands — shall we
call it done, or is there something on it you'd change?" — a decision put
to the user with nothing to select, which is the one shape this framework
refuses everywhere else, and which re-opens the round they had just
closed.
Found by eval/judge.py: shown two transcripts of the same notebook
without being told which was which, a judge picked the older one and gave
this ending as a reason. The register check could not see it — it looks
for drawn controls and framework vocabulary, and an open question is
neither.
The "at most one offer" line now says what an offer is: it names the
thing you would do next.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Preflight: control absent is not control disconnected
A real run on Linux ended preflight saying "Everything's set up ... and I
can open pages in your Chrome and check them myself", retracted the
second half later — "Correct — I can't" — and only then raised the
extension: "I'd have to ask you to set that up".
Three gaps behind that:
- The ladder assumed the probe runs and answers. When ToolSearch returns
no schema there is no probe, and launching a browser cannot create one.
That is now its own rung, distinguished from a schema that loads and
answers "not connected", which launching and re-probing can still fix.
- The offer to install was written inside a code block as somewhere the
ladder ends up. It is now an instruction with its consequence: raise it
in preflight or not at all, because raising it later lands the request
in the middle of a decision — the thing this step exists to prevent.
- Nothing forbade a summary sentence that joined opening a page to
checking one. They are the two rows of the table above and have
different requirements, so until a page has been driven the summary
says what was installed and nothing about checking.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Never resolve a fork yourself and report it as applied
A run opened with "Two calls before I build the config page", laid out
the evidence for both, offered nothing to select, and followed with
"Applied: relative precursor tolerance now defaults to 0, fragment
tolerance to 20 ppm" — then spent a later turn explaining that at 20 ppm
the best match explains 5 peaks rather than the notebook's 33.
Every word of the evidence was right and the user decided nothing.
Naming something a decision and then making it reads as consultation and
works as an announcement.
Found by eval/judge.py on a blind pair; register.py scored the same
build 0 findings, because the shape has no tick-boxes, no framework
vocabulary and sits under the word budget.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Take the user to the extension install, don't defer it
A run ended preflight with "Happy to point you at the setup for next
time if you want it", turning a one-click fix available right then into
homework. Claude cannot finish the install — the extension comes from the
store, in the user's browser, under their claude.ai account, so the click
and the sign-in are theirs — but everything up to that point is not.
Preflight now opens the install page and says the one thing to click.
Where the tools load but report nothing answering, the run carries on
with capture while they install and re-probes before the first design
round. Where the tools have no schema at all, nothing done now can help
this run, so it waits and hands them the restart: "one click to add it,
then start me again — nothing's been decided yet".
Those two failures looked alike and are not, so CONTEXT.md now
distinguishes control absent from control disconnected, and defines
browser control against merely opening a page.
The prohibition on naming the extension is narrowed rather than dropped:
never to explain a limitation, which tells a mass spectrometrist about
this framework's wiring and gives them nothing to do; only inside a
request, where it comes with an action and a reason. After a decline it
is not named again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* A browser that answers is not necessarily your browser
A run drove a browser successfully for a whole build, pointed it at
localhost:8577, and read a different Streamlit app on that port —
reporting its pages back as the user's. Chrome was on Windows, the shell
was Linux, and the extension reported the browser as not local. Nothing
in the tab context distinguishes the two cases: it reads
"New Tab" chrome://newtab/ either way.
So confirming control now has a second half. Serve a token from this
machine, drive the browser to it, require the token back. A browser that
fails that is never driven — a screenshot of the wrong app is worse than
no screenshot, because it is a finding about someone else's software
delivered as theirs.
The remedy splits by what the extension says. Not local: the browser is
on another machine and there is no second one to switch to, but the app
binds every interface, so the question is which address that host can
route to — try one and re-marker. Local but still failing: the extension
is answering for a browser not signed in under the same account, which
the probe's own error names.
That run ended "Nothing to fix from here", which is the third failure.
Falling back to the headless browser is fine; declaring a dead end is
not. The cost is stated once in the user's terms, with the one thing
that would change it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Factor browser setup out into connect-browser-control
Preflight step 4 had grown to roughly 150 lines — find a browser, launch
it, probe, confirm by driving, prove it is this machine's browser, split
the remedy by locality, offer the install, state the fallback. It was the
largest single block in the orchestrator and none of it is specific to
notebook ports: anything that needs a driveable browser needs all of it.
It is now its own skill, and the spine points at it.
Two things the split let us state that were not stated before.
Chromium only, and no Firefox. The native messaging host Claude Code
registers declares a `chrome-extension://` origin, which Firefox cannot
load, and it is registered for Chrome, Edge and Chromium alone. So the
answer to "is there a Firefox extension" is no, and not one this
framework can supply.
Open the app in the browser you drive. A user whose default is Firefox
was shown the app there while the framework checked a Chrome rendering —
two pictures, and a header truncated in one sits fine in the other. With
control confirmed, the app opens where the checks happen; without it, in
their default, with no implication that anything can see it.
That also fixes a rule this session introduced: "opening a page starts a
browser, so re-probe" assumed what opened was Chromium. `xdg-open` opens
their default, so the re-probe now happens only when what opened was the
browser that was found.
Rules and probe mappings moved with the text, since rules.json is keyed
by skill directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Name the browser to open, the cache to clear, the offer to make
Three defects, two of them reported from a real run.
A user who had just installed the extension in Chrome was shown their app
in Firefox. The rule said to open the app in the browser being driven but
gave no command, and every obvious command — start, open, xdg-open —
opens the default. It now names the executable, using the two forms that
were already verified for launching it.
Insight keys its cache by config hash, so a panel can render unchanged
after a real edit. That was one row of a table headed "component rules
that are not obvious"; it is now at the moment the symptom appears, in
the panel round, as the first suspect rather than a footnote.
And this skill was supplying the very sentence handover.md now forbids:
"the page reads well as it is — shall we call it done?" A blind judge
picked an older build over a newer one partly for that ending, and the
rule against it went into handover.md without noticing where the wording
came from. The offer now names what would happen next, and points at
handover.md for when the round is also the last turn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Consider only the attached browser; never explain it to the user
A real run told a user "the browser reachable from this session lives on
another machine, so I can't look at your finished pages myself... running
this on the machine your browser is on is what would change that."
It obeyed every rule the skill had -- cost stated once, in their terms, the
extension unnamed -- and still explained the deployment topology of a coding
assistant to a mass spectrometrist, then offered relocation as the fix. The
prohibition named a noun (the extension) when the rule was about explaining
a limitation at all, so the shape survived by swapping in another noun.
Claude Code is routinely a shell on one machine and a browser on another,
and the user picks neither. So:
- Where the attached browser runs may route the next command. It is never
a finding and never reaches the user.
- The marker check asks whether the browser can reach this host, walking
candidate addresses, not whether it is "ours".
- With control, the app opens by navigating the browser already held --
the user's own window. No executable, no default-browser lookup, which
makes the Firefox failure structurally impossible rather than forbidden.
- The URL the user is given is always localhost:<port>.
- No preamble. Rounds ask them to look at one named thing when it matters,
which is what a design round sounds like anyway.
Also corrected two claims of my own. The native messaging host is registered
for seven browsers (Chrome, Edge, Chromium, Brave, Arc, Opera, Vivaldi), not
three -- the registry disagreed, and the old claim would have skipped a
working Brave or Vivaldi. And the browser that rendered a different Streamlit
app was most likely an earlier build of the user's own, not "someone else's
software"; the cause never mattered, only that the pixels were different.
coherence.py found five contradictions across five passes, all introduced by
adding a rule beside an existing one without re-reading it -- including an
exemption clause that leaked past its trigger. Clean on the fifth.
199/199 rule placements present, 0 retired claims live, 25/25 browser rules
probe-reached, 0 corpus leaks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Never narrate preflight
Measured in 10 builds of 19, across fifteen wordings: "Preflight - probing
environment in parallel", "Now the browser side of the preflight", "Preflight:
Python, browser, dependencies". Each hands a mass spectrometrist a list of this
framework's own steps, and the word means nothing to them.
register.py could not see it -- `preflight` was absent from its internal
vocabulary, so every one of those builds scored clean. Adding it (and
`browser control|setup|side`, keeping bare "browser" legal) turns four sampled
builds from 0 findings into 1-3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Fix a stale count and reconcile two definitions of "stage"
Both found by coherence.py, both pre-existing.
"Five habits:" heads six bullets -- a09c6f7 inserted one and left the count.
"A stage is something the user would name -- pulling the analysis out, checking
it against their numbers, building the app" gave capture two nameable stages,
while the Stages table gives it one and the announcement rule says "each stage
except stage 5". Anchored the definition to the table so the two agree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Preflight's silence must not swallow the install request
A live run on the user's own notebook read: "Setting up - a minute, then I'll
ask about your notebook." then "Ready. What's the path to your notebook?" and
never offered the browser extension at all.
That is my regression. The narration finding got answered with prohibitions --
"neither state is ever explained", "no preamble", "never narrate this stage" --
stacked in two skills, and they reached past narration into the one turn
preflight is supposed to produce. writing-skills says exactly this: a wrong-shape
failure answered with prohibitions gets over-applied, and the fix is a positive
recipe with a required slot.
So both are now recipes rather than bans:
connect-browser-control gains a four-row table that IS the whole user-facing
output of browser setup. Three rows are silent. Row two -- no schema, or "not
connected" -- is the install request, marked required, with the silence rules
explicitly not reaching it: a run that stays quiet there has silently decided
on the user's behalf that they would rather not have been asked.
notebook-to-webapp states preflight's output as at most two turns: an optional
"setting up" line, and the install request whenever step 3 earns one.
No build could have caught this. Every corpus build runs on a Windows box with
Edge and the extension present, so the missing-extension branch is unreachable
from the harness -- 25 ticks of green on a path the corpus cannot walk. The
followability case now probes it directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Never settle for the headless browser without being told to
A live run drove a browser for an entire preflight that the user could not
see, said nothing about it either way, and carried on -- while a Chromium
browser sat installed and extension-free at /usr/bin/google-chrome, one click
from working. The user asked twice what was happening before anything
surfaced.
The premise that broke: attached is not theirs. The extension answering proves
a browser exists somewhere reachable, not that anyone is sitting at it, and
only the marker separates the two. CONTEXT.md said otherwise and is corrected.
connect-browser-control now ends in exactly two states -- control confirmed, or
no control after an explicit decline. There is no third ending and a run never
reaches the gate's headless browser on its own judgement:
- silence is earned by confirmed control or by a decline, nothing else
- the browser search is ungated: it runs whenever control is not confirmed,
which is the case an earlier gate excluded and the one that most needs it
- a candidate you are blocked from testing is a candidate that failed; the
run that reported "I can't yet trust it" and promised itself a retest was
inventing a state this skill does not have
- the ask is scaled to the road -- the click, the click and a restart, or a
browser to install
- preflight stops there until they answer, and only an explicit no is a
decline; whatever they reply, the probe is read before their words
- on a decline, one line, and it is about them: "I'll ask you to look at a
page or two as we go"
- asked directly, answer fully -- the silence rules govern what you
volunteer, not what you answer
notebook-to-webapp's preflight goes from two user-facing turns to three, and
the notebook question waits behind the ask.
Retired: the search runs only when nothing is attached; the silent rows are
silent because nothing in them is theirs to do; preflight produces at most two
user-facing turns.
RULES 210/210 . RETIRED_CLAIMS 0 live . GENERALITY 0 leaks . CONFORMANCE 3/3
FOLLOWABILITY 14/14 unreachable, 10/10 install, 11/12 preflight
COHERENCE found two contradictions in the rewrite, both closed: the table
collapsed "not connected" and no-schema into one row when they take different
asks, and no-schema skipped the search that its own last row depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Presence cannot be measured, and control can go stale silently
A fellow agent's debug report on a preflight that took 25 calls and two wrong
user-facing questions where 8 calls and 0 questions would have done. Its
findings were audited against the record, most held, several did not, and
probes run on the reporting host settled what neither of us could read.
The standing constraint that shaped the rest: the only browser this framework
drives is the local one.
WHAT WAS MEASURED, not reasoned
switch_browser EXCLUDES the browser you are already attached to. One browser
connected returns "No other browsers available to switch to" in under six
seconds; two browsers with the user sitting at the attached one timed out at
~124s three times running while that user watched their own Chrome and reported
"Nothing appears..". Its own description claims it prompts "every Chrome browser
with the extension installed". It does not. So nothing available proves a human
is in front of a window, and the skill no longer implies otherwise.
Confirmed control goes stale, and it fails in the SUCCESS direction. A user
closed their browser; the device left the account list on its own, the session
was silently re-attached to a browser on another machine, and navigate went on
returning "Navigated to <url>" on every call while rendering where nobody was.
pgrep counted three live Chrome processes after that device had gone. Only
serving a page and failing to read it back revealed any of it.
get_page_text refuses a body under ten characters. Bisected on a live server,
same tab, same Content-type: abcdefghij passes, abcdefghi fails, and padding the
nine with newlines or spaces still fails -- the threshold is on trimmed text. A
run lost its marker to a nine-character token and diagnosed the extractor.
Display names are positional. The same deviceId was "Browser 2" one day and
"Browser 1" the next; the list renumbers on membership change.
THE MARKER, rewritten
- loopback is the only candidate; the routable walk is deleted. A routable
candidate has never returned a token in any recorded run, and the
classifier's treatment of named routable addresses differs by host
- --bind 127.0.0.1 written out, because http.server DEFAULTS to 0.0.0.0 and
dropping the flag widens the bind rather than narrowing it
- curl from this shell before driving, or a dead server and an unreachable
browser are the same error -- one run wasted four calls navigating at a
corpse after a port collision
- the token is at least ten characters, and the port is genuinely random;
three recorded builds all used 8791
- get_page_text is named and on the ToolSearch line, matching out of the page
BODY: tab context echoes back the URL you requested, token and all, after a
navigation that totally failed
ENUMERATION, read but never shown
list_connected_browsers is read to route and never rendered. Its description
mandates an AskUserQuestion listing every browser with its deviceId, which is
the topology retired at t187; the mandate lives in that description alone and
select_browser carries none of it. Select by deviceId, never by name, and note
that nothing in the list says which browser you are attached to -- a run that
reported "attached to the wrong one" had inferred it by elimination from a list
holding exactly one non-local device.
RECOVERY, which is new
Control is confirmed once and then used; there is no re-check between rounds.
When a page comes back wrong, re-enumerate rather than re-probe -- tabs_context
answers healthily for whatever you are attached to now. The repair is one action
and it is theirs: reopen the browser. Measured, the device returns on its own
with no reinstall and no sign-in. Then select by deviceId and marker again.
Also: check the port is held by the process this run launched before pointing a
browser at it.
WHAT THE REPORT GOT WRONG
Its headline recommendation, enumerate before any user-facing ask, is refuted by
its own connectedAt values: the browser it wanted appeared 19m33s into the
preflight, so the extension question was correct and enumeration then would have
changed nothing. The real defect was five calls later, re-probing where it
should have re-enumerated. "Compound commands are denied wholesale" is refuted
by its own timeline -- four were allowed and two of five denials were plain
single commands. Its 5.6 and 5.7 are rejected outright; 5.7 attacks a placement
that does not exist and would resurrect a claim retired at t190.
Retired: the routable candidate walk; --bind 0.0.0.0; the not-local branch that
blessed driving a browser the extension reports as elsewhere; "a browser that
loads what this host serves is on the user's side of the wire" (sshd listens
here, so loopback proves nothing about location); "read the page" naming no tool.
Coherence found four contradictions across three rounds and three were
pre-existing: the marker was still said to separate presence twenty lines above
the correction saying nothing can; "no sentence in preflight saying you will be
asking them to look" forbade the mandated decline line; and the same retired
behaviour was numbered third in one place and fourth in another.
notebook-to-webapp-preflight had been stuck at 11/12 since t190 and the probe
was right: on Windows the default browser IS the Edge the run just launched, one
window not two, and the re-probe rule never said so.
RULES 225/225 . RETIRED_CLAIMS 0 live . GENERALITY 0 leaks . CONFORMANCE 3/3
COHERENCE 0/3 runs . FOLLOWABILITY 17/17 unreachable, 10/10 install,
7/7 lost (new case), 12/12 preflight
Known hole, accepted deliberately: nothing loads a page between preflight and
the user's app, and the port check only sees this host, so a browser elsewhere
resolving localhost to its own port still shows another app's pixels.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Name the browser, then watch for the device
A live preflight on the previous commit's text got further and still failed
twice: it opened the extension store page in Firefox, and then waited to be told
the extension had arrived rather than looking. Its user had to type "continue".
The session was interviewed, and both failures turned out to be omissions rather
than violations -- it did what the text said, and the text was wrong.
THE STORE PAGE
It had Chrome. It searched the family with command -v, found
/usr/bin/google-chrome, launched it, probed -- and then opened the store page
with xdg-open, which resolved to Firefox, where a chrome-extension:// origin
cannot be installed at all. It then told its user "I've opened a page in your
Chrome" on no evidence.
Two sentences licensed that, and neither was wrong on its own: this skill's
"open it the same way you open any page", and the orchestrator's open/drive
table offering start/open/xdg-open with no caveat at the point of use. The
sentence that should have stopped it -- the one whose worked example is this
exact failure -- opens "With control," and was read as scoping the paragraph to
opening the app later.
So the store page now opens by naming the executable, in both shell forms, and
the orchestrator's table gains the row that decides it: the default is right
when any browser will do, wrong when one specific browser must receive the page.
Measured, naming the executable hands the URL to an already-running instance
rather than starting a second one -- 94ms return, a /favicon.ico line in the
served page's log that curl never produces, and a process count rising by
exactly one renderer.
THE WATCH
Between opening the store page and the user's "continue" the run made one call.
It never read list_connected_browsers. That device's connectedAt proved it had
connected before the user typed anything, so the entire wait was invented.
Nothing had forbidden a loop. The wording simply described enumeration as a
response to their reply -- "whatever they reply, re-enumerate" -- which pinned it
to a turn boundary instead of to the clock. Now: the ask stops the notebook
question and anything that moves the run past them, but not setup that has to
happen regardless. The gate's headless browser install was already sequenced
after the ask, so it runs during the click and nobody had taken that overlap.
Then one look at the end of preflight, then every fifteen seconds until the
device appears or they speak.
Scoped, because the watch as first written was not: on the road where ToolSearch
returns no schema there is no list to poll, which is exactly why that row of the
ask asks for a restart.
WHAT WAS FALSIFIED
"Edge ships as the default on Windows, so start <url> there opens the browser you
already launched" -- retired. This Windows machine, with Edge installed, defaults
to Firefox. Worse, that paragraph caused the failure above: the run on Linux read
it as licence to expect xdg-open to resolve to the Chrome it had just launched.
What replaces it carries the lookup commands and sits on the decline road, where
a run standing at the ask never reads it.
tabs_context_mcp answers for the MCP tab group, not for the browser. Measured
byte-identical after a launch whose page had demonstrably been fetched -- so it
is a false negative for confirming anything opened by other means, and it is
what the next run would have reached for. Nothing confirms the launch now; the
watch confirms the outcome.
The native-messaging-host paths list registrations, not installations: a Windows
box registering Chromium and Vivaldi had neither, only Edge; a Linux box carried
manifests in five browser directories with one browser on PATH.
The confirmation is one navigation again. The screenshot goes on the marker page
-- computer returns "Successfully captured screenshot (1470x751, jpeg)" on the
served text/plain page with the token legible -- so the separate ordinary-page
step a live run skipped as redundant is deleted rather than restored.
Preflight's three-turn budget had no home for acknowledging a click, so a run
invented a fourth turn, "Setup's done." It was true; the marker had run and
passed. The acknowledgement now rides on the next question -- "Thanks -- what's
the path to your notebook?" -- and is not a turn of its own.
COHERENCE, and the failure mode it exposed
Six findings over four rounds, and four of them were one pattern: amending a
rule leaves every sentence that SUMMARISED that rule asserting the old one.
Worth naming, because this tick created three of them and every other check
stayed green throughout.
- the opening still said "preflight does not move past it" after the ask was
scoped to hold only the notebook question
- "Both launch forms above" silently changed meaning when the store-page
commands became a second pair of launch forms above the same sentence
- the orchestrator still called it "the one that stops this stage" beside the
new rule saying it stops the question and not step 4
- the orchestrator named two things that reach the screen while its own table
mandated three -- a reader would have delivered the Upload, Configure and
Results rounds as interviews
Two were older. The skill disclaimed presence in three places and then called
the confirmed browser "the user's own window"; loopback reach is now stated as
what it is -- the closest thing to their window anything here can establish,
treated as such on purpose, never a claim that anyone is sitting at it. And the
setup-summary rule read as licence for a summary that the turn budget does not
allow; it now says it governs the wording of one licensed elsewhere.
Two of the instruments were wrong as well, and both were found by using them.
eval/coherence.py died on an arrow in the judge's prose, because Windows stdout
is cp1252 -- and it died AFTER the judging was paid for, losing the finding
rather than the run.
eval/generality.py matched corpus artefacts case-sensitively, so a corpus stem
survived being lowercased into a filename. EUBIC_Task3_Quant had been sitting in
notebook-to-webapp as `eubic_task3_quant.py` behind a green check: an agent
handed that notebook could read its own expected output filename out of the
skill it was being measured against, which is the exact overfitting the check
exists to prevent. The match is now case-insensitive, which flags that line and
nothing else across all seven skills, and the example carries placeholders.
RULES 236/236 . RETIRED_CLAIMS 0 live . GENERALITY 0 leaks . CONFORMANCE ok
FOLLOWABILITY 13/13 install, 20/20 unreachable
COHERENCE connect-browser-control 0/3 runs, clean
SKILL_DEBT 24441 -> 26432: ten rules added, five claims retired.
Left open deliberately: the watch has no clock ceiling, so a user who walks away
after the ask leaves a run looking every fifteen seconds. And the executable
launch is measured in one configuration -- same user, same profile, already
running -- not in the class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Insight is the first solution, and a missing one has to say so
A live session built a results page with no OpenMS-Insight components in it.
Interviewed, it turned out to have done nothing wrong: it attempted the import,
got ModuleNotFoundError, and fell back exactly as this skill told it to. The
framework had no way for it to do better.
WHAT WAS MISSING
There was no install step anywhere. openms-insight is not in the template's
requirements.txt -- correctly, per the design doc, which says it belongs to
generated apps and not to the template -- but no skill in the chain ever put it
in the app's either. It worked only on machines whose venv already carried it,
which is every machine the corpus has ever run on.
So build-insight-dashboard now establishes it before mapping anything: the line
goes into the app's requirements.txt, the venv is synced, and the import is
confirmed. Verified by importing, not assumed.
INSIGHT FIRST MEANS A LOOKUP, NOT A JUDGEMENT
ADR 0002 pinned half the output contract -- files, columns, identifiers -- and
left `role` an undocumented free-form string, while the dashboard stage was told
to "map each role to a component". Both ends were unpinned, so "no Insight
component fits" was a judgement reachable in one step.
The vocabulary is now fixed in capture-notebook-workflow (table, mirror, peakmap,
chromatogram -- the four in use, each with an exact component), and
build-insight-dashboard carries the table that maps them. A role in it gets that
component; there is no choice to make. A step may coin a role outside it, and
must say it has.
The run that prompted this had roles `table` and `mirror`. Both had components.
Nothing failed to fit.
THE FALLBACK HAS TWO REASONS AND NAMES WHICH
Retired: "Where no Insight component fits, fall back to pyopenms-viz and say so
in the wireframe." It could not tell a design judgement from a missing package,
and what it produced was worse than a miscategorisation. The wireframe that run
showed its user read:
"The mirror plot is drawn with pyopenms-viz, the same backend your notebook
used."
True, and it attributes the choice to continuity with their notebook. The
environment fact appears nowhere in the transcript. Its own later self would
have read that as a considered design decision.
Its diagnosis of the cause is the part worth keeping: the register rules leave
this finding no channel. notebook-to-webapp forbids naming framework plumbing;
verify-webapp-usability makes the gate internal. "The headline component is not
installed" is exactly what every one of those rules pushes out of a turn, and
none of them says where to route it instead. So the reason now rides on the
wireframe as a required clause -- one turn, not two, and the slot cannot be
filled without naming which of the two reasons it was.
THE GATE WAS NEGOTIATED, NOT PASSED
NATIVE_PANEL_SELECTORS counted Plotly, Vega and Pyplot but not stDataFrame. That
bites only the table role: a plot falling back renders Plotly and counted, a
table has no plot to fall back to. The run read a real 9/10 on a page missing its
table, then changed --expect-components from 2 to 1 and recorded a green 10/10.
stDataFrame now counts, the panel count comes from the link graph, and lowering
it to reach PASS is named as the one thing a run may never do.
AND THE WIREFRAME MAKES SECTION 1 UNSKIPPABLE
That run read lines 1-120 of a 195-line skill and produced no link graph at all
-- "not one line reached the user or my own reasoning" -- going straight to the
wireframe. Every fix above lives in section 1, so every panel in the wireframe
now names the role it came from and the component it uses. You cannot fill that
in without having done the mapping.
APPS DECLARE WHAT THEY IMPORT
The two most recent builds ship apps whose dashboards import openms_insight while
their requirements.txt never mentions it; a clean install of either dies on
import. Three earlier apps carry the line, so this stopped happening rather than
never happened. Stated generally in scaffold-workflow-app, because Insight is the
instance that got caught rather than the whole problem.
THREE DOCUMENTS DISAGREED
add-visualization claimed requirements.txt contains openms-insight -- false of
this repo, and checking requirements.txt is the obvious verification, so that
line would have actively misled a reader. The design doc pins openms-insight
0.2.0 in an environment table. CLAUDE.md was the only accurate statement and is
now the complete one.
COHERENCE
build-insight-dashboard and capture-notebook-workflow both come back coherent.
scaffold-workflow-app reported a different contradiction on each of three runs,
and two were real and pre-existing: one section mandated editing the root
test_gui.py page list while another said the template's tests are not yours to
change (the seam is now named -- tests/ is a directory, test_gui.py is a root
file), and an anecdote named three files then called them "the three files and
the fourth". Both fixed. A document that yields a new finding per run has a
backlog, and clearing it belongs in its own pass, not this one.
RULES 244/244 . RETIRED_CLAIMS 0 live (32) . GENERALITY 0 leaks . CONFORMANCE ok
FOLLOWABILITY 7/7 build-insight-dashboard-unavailable (new case), 22/23 main
SKILL_DEBT 26432 -> 27917: eight rules added, two claims retired.
Left open, deliberately. The main insight case sits at 22/23 missing "three axes
named", consistently over two runs; there is no before-measurement, and the
likeliest cause is that widening that case from 19 to 23 probes thinned the
answer rather than the skill getting worse. And counting stDataFrame could count
an incidental show_table() as a panel and so mask a missing one -- the 50x50 size
filter is what stands between those, and it is unmeasured.
The new followability case exists because nothing in the corpus could have caught
this: the failure only occurs on a machine without the package, and the corpus
runs on a machine with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Five corrections the first live debriefs found
The debrief harness ran for the first time and returned 40 items across two
builds. Four of its confirmed findings indicted edits made the same morning,
and each is a correction: a statement that was false, or a rule that made a
legitimate build impossible.
chromatogram no longer maps to LinePlot. Verified against the component's own
docstring, which reads "Interactive stick plot component" and "Stick-style peak
visualization (vertical lines from baseline)". The class name was the trap: a
build read it, mapped an elution profile onto it, and drew 156 points as 156
vertical bars. Insight ships eight components and none draws a continuous
trace, so chromatogram is the role that legitimately has none. It takes
pyopenms-viz with the reason "no component fits". capture-notebook-workflow's
vocabulary block still promised that a role in the list had a component waiting
for it, and now does not.
Hidden and missing are different findings. A two-tab layout puts a panel behind
an inactive tab whose iframe reports 0x0, so the gate's total fell 5 to 4 --
against a rule forbidding exactly that number. The count is now scoped to one
screen and each tab gated separately; what is still forbidden is lowering it
for a panel that failed to render.
The screenshot half of the marker check had no verdict. The recipe takes a
screenshot on the marker page, but the table under it read on the token alone,
so a session that passed the marker, drove pages, read their text and had every
computer call fail landed on "it can reach you, say nothing". Reading a page
and looking at one fail apart.
A background command's description is user-facing text. The harness prints it
back on completion, so a run can keep every silence rule in its prose and still
put `Install headless browser for the gate` on screen. Name the command for
what the user is waiting on.
A failing background command is not narrated. One build printed `Background
command "Restart the app" failed with exit code 1` on three separate screens
and the reader flagged every one.
Found by pointing register.py's vocabulary rules at real build screens for the
first time -- they had only ever graded hand-written transcripts, so rules that
existed for dozens of ticks had never been asked.
RULES 246/246 . RETIRED 0 live (33) . CONFORMANCE ok . GENERALITY 0 leaks
COHERENCE build-insight-dashboard and connect-browser-control both coherent
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* run_python spawns the interpreter running the app, and its tests say so
Two builds on two different notebooks were put in an impossible position by
the repository, not by any skill. scaffold-workflow-app sanctions exactly one
edit inside src/workflow/ -- run_python() must spawn sys.executable rather than
the literal "python", because a venv's interpreter is not on PATH under that
name. Making that edit breaks two assertions in
tests/test_command_executor_run_python.py, which pin the literal. And the same
skill forbids editing the template's tests to match a change you made.
So a build that follows the guidance correctly ends with a red suite it is not
allowed to fix. Both builds found it independently and reported it in their
debriefs; it is the first implementer-side pitfall to reach the recurrence
threshold, at two builds across two notebooks.
Both halves of the fix had been sitting uncommitted in the working tree since
this morning, so every clone carried the unfixed executor together with the
tests that assume it. Committing them is the whole repair -- the guidance was
right and the clone was inconsistent with it.
src/workflow/CommandExecutor.py "python" -> sys.executable, 2 sites
tests/test_command_executor_run_python.py the two assertions that pin it
7 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* An anecdote about a mistake is not a rule against it
A build cloning the current skills opened with "Preflight's done. Let me read
your notebook." This skill already contained that mistake, written up: a run
that wrote "Setup's done." and explained honestly why -- two visible turns of
trouble had just happened and silence read as the request having gone nowhere.
The write-up stayed an anecdote. The rule beside it says what is never said is
anything *about control*, and a stage of setup finishing is not about control,
so nothing forbade the turn and the next run made it one word worse by reaching
for the internal name.
Setup finishing is not news -- the user did not know it had started. preflight,
marker, the gate and browser name machinery they were never shown.
Measured: this is the first build to clone t194 and leak anyway, and it leaked
through the gap that rule did not cover rather than through the one it did. The
same build's background command reads "Install dependencies" and the whole run
is otherwise clean, which is t194 holding.
RULES 247/247
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The opening line is a slot, and three builds each found a way to fill it
Three builds on three different notebooks opened with a status line accounting
for setup, each running with the rules the previous one had earned:
t2i "Now preflight -- Python, dependencies, and browser setup, before I ask
you anything."
t1p "Preflight's done. Let me read your notebook."
t3q "Control confirmed -- nothing to ask. Now the gate's browser and a look
at the notebook."
The second ran under a rule against narrating control. The third ran under that
plus a rule against the word preflight, and said "Control confirmed" instead.
Each rule closed a phrasing and the next run reached past it, which is what a
vocabulary rule does to a pressure it does not name.
The pressure is that setup took real time and finished, and saying nothing
feels like saying nothing at all. The user did not know it had started, so its
ending is not news. Naming the slot rather than the words: there is no turn
before the first question.
Measured against t194, which named a channel -- a background command's
description is user-facing text -- and held without anyone touching its
wording: the same build renamed its command from "Install headless browser for
the gate" to "Install browser for page checks" unprompted. Structure held where
vocabulary was evaded.
The leak is confined to screen-000 in every case; nothing after it narrates.
RULES 248/248
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* A stage gets an outcome line, and the old count was false of its own example
Two builds on two notebooks followed "one progress line per stage" exactly and
reported the two longest silences in their runs. Capture covered the extracted
script, the golden test and a real transcription bug on a single line; scaffold
covered the workflow class, four pages, registration, the template cleanup,
three test lists and the dashboard on another. One of them stated the gap
directly: "nothing in it says what to do when a single stage runs long."
The count was already wrong against this skill's own worked example. That
example gives capture two lines -- it opens, and then "Verified it reproduces
your notebook", a result the user can check -- while the sentence above it said
one. Both runs followed the sentence and dropped the line the example shows,
which is the line that would have broken the silence.
The outcome line is not narration. It is the answer to the question the silence
makes them ask. Where a stage genuinely produces nothing checkable until it
ends, it keeps its one line and the next question closes the gap; a stage
running long is not a reason to say more about the stage.
This is the counterweight to today's other five edits, which all removed
narration. It was found by asking the two agents that built the apps, and no
output check could have produced it: both builds scored clean and the silence
is invisible in an artifact.
RULES 248/248 . RETIRED 0 live (34)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* t198 forbade a line the framework is supposed to write
Written two hours ago as "there is no turn before the first question. Not a
short one, not a framing one." That contradicts notebook-to-webapp, which
sanctions exactly one: "Optionally one line saying you are setting up --
'Setting up -- a minute, then I'll ask about your notebook.'"
The next build settled it. Cloning t198 and that sanctioned line together, it
opened with the sanctioned one and produced the first clean opening measured:
no preflight, no control, no gate, no browser, and register clean across every
screen. The rule I wrote would have deleted the line that fixed the problem.
So the rule is about direction, not existence. All three failures reported work
already done -- "Now preflight -- Python, dependencies and browser setup",
"Preflight's done", "Control confirmed -- nothing to ask". The line that works
looks forward: it says what is happening and what comes next, and names no
machinery. Nothing reports that setup finished.
Measured openings, in order:
"Now preflight -- Python, dependencies, and browser setup..." backward
"Preflight's done. Let me read your notebook." backward
"Control confirmed -- nothing to ask. Now the gate's browser." backward
"Setting up -- a minute, then I'll ask about your notebook." forward, clean
RULES 248/248 . RETIRED 0 live (34)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The outcome line needed a bound; without one it doubled the count
t199 permitted a second progress line per stage -- the outcome line this
skill's own example already showed -- to fix the two longest silences two
builds had reported. The first build to run with it wrote eleven progress
lines where the same notebook had written six the build before.
0904-2141-t1p t199 absent 6 progress lines
0905-0032-t1p t199 present 11 progress lines
Same notebook, same persona, one variable. Among the eleven was "Now
test_gui.py -- all three of its lists.", which is verbatim the line the bad
column of this skill's own example is annotated for: a step that only exists
because this framework has a rule about it is not a step the user is waiting
on. The permission was read as licence, and it brought back the failure the
whole rule exists to prevent.
The bound is what makes it a permission. Five stage openings plus an outcome
line for each stage producing a result before it ends -- in practice four to
six for a whole build, against four in the worked example. A seventh means
steps are being narrated.
This is the second time today a rule of mine was evaded or over-read, and both
times the fix was a countable structure rather than a description. t194 named a
channel and held. t197 named a word and was walked around in one build. t199
described a permission and was stretched in one build; t200 gives it a number.
RULES 248/248 . RETIRED 0 live (34)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Trouble spends a budgeted line rather than earning a new one
Two builds on two notebooks reported the same inversion. One stated it exactly:
"the stretch where I had least to report was the stretch where I was busiest,
and the user saw nothing for longest precisely when something was wrong." The
longest run in one of them was a held port, an unsupported widget_type, and a
server that exited three times with no traceback.
Every rule here correctly forbids narrating any of that, and "a fault with no
user-visible symptom is a reason to say less" then makes the busiest stretch the
quietest by construction. Saying you are still going is not describing the
fault; it is the one fact the silence makes them guess at.
The form matters more than the content here. Two edits today added a permission
and both inflated what they permitted -- t199 doubled the progress-line count in
one build, and t197's forbidden word was walked around in one. This one spends a
line from the existing four-to-six budget instead of adding to it, so there is
nothing new to stretch.
Measured trade, three builds, same instruments:
no permission, no bound 4 silence items, 6 progress lines
permission, no bound 2 silence items, 11 progress lines
permission, bounded 4 silence items, 5 progress lines
The bound restored the count and the silence came back with it. This spends one
of those five rather than reaching for a twelfth.
RULES 249/249 . RETIRED 0 live (34) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The same inversion lives in the design round, and t201 only fixed one skill
t201 gave a stage in trouble a budgeted line to say it is still going, in
notebook-to-webapp. The next build reported the identical inversion from
build-insight-dashboard instead:
"each iteration was edit -> clear the Insight cache -> restart -> render ->
read the screenshot, and each one ended in a fault I am specifically told
not to describe -- phantom-wrapped rows, truncated headers, a
column-not-found on the sort field, a failed restart -- so the rule that
keeps instrument language off the screen also guarantees that the stretch
with the most going wrong is the one with nothing said."
Four builds have now reported it, in three skills. This is the standing
obligation in eval/README.md: apply a correction everywhere the claim lives,
which six earlier ticks failed to do.
Same form as t201 and for the same reason -- one line, spent from the
four-to-six budget rather than added to it. Two edits tonight that added a
permission had it stretched within a single build.
RULES 250/250 . RETIRED 0 live (34) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Setup is not a stage, said out loud this time
Two consecutive builds on two notebooks, both carrying every rule in this
repository, wrote a progress line for setup:
Now the browser control probe.
Control confirmed -- token and screenshot both came back. Now the gate's
headless browser.
Now step 3 of setup -- browser control.
"Step 3 of setup" is the tell. It numbers something the user never knew had
steps, which means the run had modelled setup as a stage with progress in it.
The rule already forbade this. The stage table at the end of the file has five
rows -- capture, interview, scaffold, dashboard, verify -- and the sentence
above it says the stages are the five and nothing else is one. Setup is not
there. Both runs read past the implicit form, which is what an implicit form
does when the thing it excludes genuinely takes time and every other
time-taking thing gets a line.
So it is explicit, with the check attached: before writing a progress line,
name which of the five it belongs to; if the answer is "setup", you have your
answer.
This is a violation rather than an omission -- the third distinct phrasing of
the same turn tonight, after "Preflight's done" and "Control confirmed --
nothing to ask". The first two were answered by banning words and were walked
around within one build each. This one names the category instead.
RULES 251/251 . RETIRED 0 live (34)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Two concrete defects the debriefs promoted, both crashing or misreading
preset_buttons is the one ui.* method the framework already calls
------------------------------------------------------------------
Two builds on two notebooks added self.ui.preset_buttons() to the end of
configure(). One rendered the presets twice; the other crashed the Configure
page on duplicate widget keys. The skill stated the fact -- they auto-appear via
parameter_section() -- and never said not to call it, and every other ui.*
method IS theirs to call, so the inference is the natural one. Named now, with
where to look when presets do not appear: presets.json or the workflow name in
it, never a missing call.
Port ownership cannot be read from the PID
------------------------------------------
A venv's python re-execs as the shared base interpreter, so another build's
Streamlit server has a byte-identical executable path. One build confirmed a
listening PID and moved on. Another, after a failed restart, took what it
thought was a free port and got a 200 back from a different build's app still
running on this machine. Both were about to judge someone else's page and call
it their own.
The rule to verify the port already existed; what was missing is that the
obvious way to verify it does not work. Match on the command line, which carries
the app path and the --server.port you passed.
Both promoted at two builds across two notebooks. These are the first promoted
findings of the night that are engineering defects rather than what reaches the
screen.
RULES 253/253 . RETIRED 0 live (34) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* eval/rules.json was committed by accident and rode into nine clones
A `git add <skill> eval/rules.json` in the t198 commit staged the harness's own
rule file. Every build cloned since 67261c9 carries it -- nine of them,
including both model probes -- so a driven session could see the rules it is
being measured against, and their baselines, which describe the exact failures
the loop is watching for.
Nothing read or modified it: the clone's copy is byte-identical to HEAD and the
Haiku session's file panel merely listed it. But the surface should not exist,
and a probe run against a repo containing its own answer key is not a clean
measurement.
Untracked, and eval/ is now ignored so it cannot happen again. The nine builds
in the window are recorded in eval/README.md rather than discarded -- their
artifacts are still valid, and no evidence of contamination was found.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The marker recipe walked into the trap the framework warns about twice
capture-notebook-workflow: "`python` on PATH is often a Microsoft Store stub
that prints help and exits." notebook-to-webapp names "the Windows Store-stub
trap" twice. And this skill's own marker recipe read:
serve python -m http.server <random port> --bind 127.0.0.1 --directory <tmp>
A sonnet probe followed that line literally, hit the stub, and lost the loopback
check to it -- reported in its blind debrief as "used the bare `python -m
http.server` command shown in the skill's own marker-check example". On a
default Windows box the server never starts and the marker fails as though
control were broken, which is the one failure this check exists to distinguish
from.
Now: the interpreter you already resolved -- sys.executable, the venv's python,
or whatever capture's probe returned.
Two things worth noting about how this was found. It is a contradiction between
two skills, and nothing in the harness checks for that: rules_present verifies a
sentence exists, coherence.py reads one document at a time, and neither can see
one skill contradicting its neighbour. And it took a different model to surface
it -- the default builds resolve an interpreter out of habit and never execute
the line as written, so the defect sat in a recipe that had been read dozens of
times.
RULES 254/254 . RETIRED 0 live (34) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* A width taken from one layout was stated as a property of all of them
The rule gave a fixed content width for a 1280px window and told runs to budget
their column widths against it. Two builds on two notebooks did exactly that and
wrapped onto the phantom second row anyway:
t3q measured the real content width at ~830px
t1p measured 864px, with the wrap at a 865px sum
Each then re-derived the budget from a screenshot and repeated gate runs. One of
them named the fault precisely: "the number in the rule is a measurement from
one layout rather than a property to compute."
So the figure is gone and the instruction is to measure. This is the opposite
call from t200, which fixed a failure by giving a count -- and the difference is
what the number describes. A build's progress lines are a property of the
framework's own behaviour and can be bounded in advance. A rendered page's
content width is a property of the page, and any constant is one app's
measurement wearing a rule's clothes.
RULES 254/254 . RETIRED 0 live (35) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The trouble line was rationed by the budget it was meant to relieve
t201 gave a stuck stage one line to say it was still going, taken from the four
to six rather than added to them. I chose that deliberately: t199 had added a
permission and the count doubled in a single build, so spending from the budget
looked like the cautious form.
A build spent it and reported what that cost:
"spending it made the next silence longer, not shorter -- I said 'Still on
the results page' once during the peak-map rebuild and then held the rest of
that stage silent through seventeen shell commands because a second line
would have broken the count."
This is the first observed use of the line, and it produced the opposite of its
purpose. Rationing relief with the budget it relieves is worse than either
alternative.
One per stage, uncounted. That is still a bound -- at most five across a build,
and only for a stage actually stuck -- so it does not reopen what t199 did,
which was a permission with no number attached at all.
The same build also stacked three machinery lines above one question and named
why: "I wrote them because each fix felt like news, which is exactly the failure
the register rule names." That is the plateau, already recorded, and no rule
here addresses it.
RULES 254/254 . RETIRED 0 live (35)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ini/code/session_017KoGDZbCevZMRYSJmDbwDG
* The anti-example was the most copyable sentence on the page
This skill demonstrated the wrong way to write progress lines by printing them
in a left-hand column beside the right ones, eight against four. Three builds on
two notebooks then produced the left column's test_gui.py line almost verbatim:
skill, bad column Now test_gui.py -- all three of its lists.
0904-2141-t1p Now test_gui...
0905-0032-t1p Now test_gui...
0905-1207-t3q Now updating test_gui.py -- all three of its
hand-written lists.
The commentary under the block explained precisely why that line is wrong -- it
reads a phrase out of cleanup.md, and a step that exists only because the
framework has a rule about it is not a step the user is waiting on. Reading the
explanation did not stop three runs from copying the sentence it explained.
A well-formed sentence in a skill can be copied whichever column it sits in, and
this one was left-aligned, first on the line, and grammatical. The bad version is
now described in prose and never written out as a usable line.
This is the counterpart to t207. There, a number that described the artifact had
to go because it varied; here, a sentence that described bad output had to go
because it could be lifted. Both were teaching by example and both examples were
being used rather than read.
RULES 255/255 . RETIRED 0 live (35) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* The skills were supplying the sentences the builds kept producing
t209 removed one anti-example -- notebook-to-webapp's test_gui.py line, printed
in a left-hand column as the wrong way to write a progress line. Three builds on
two notebooks had reproduced it almost verbatim; the first build after removal
did the same scaffold work, edited test_gui.py, and wrote no line about it.
So the mechanism was checked against every forbidden term register.py knows, in
every skill. Four more written-out anti-examples turned up across two files, and
every one is a string some build has already emitted:
connect-browser-control the browser-half heading, and a background command
description naming the gate
notebook-to-webapp the browser-probe heading, the control-confirmed
sentence, the numbered setup step, and the preflight
line
Most of those I wrote overnight, as evidence in the tables documenting exactly
these failures. Four generations of setup rules were aimed at a behaviour the
file beside them was demonstrating in clean, copyable prose.
All of them are described now instead of quoted. The one line that stays written
out is the sanctioned one, because that is the sentence a run should be able to
lift.
This does not close the narration plateau. DEFAULTS/OUTPUTS, WorkflowManager,
and verify-webapp-usability's self-quoted violation have no anti-example behind
them and still stand. But the plateau's headline evidence -- the same setup
sentence recurring under four rule generations -- has a cause that is not the
rules failing to bind.
RULES 256/256 . RETIRED 0 live (35) . CONFORMANCE ok
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KoGDZbCevZMRYSJmDbwDG
* Re-navigation does not recover a wedged tab, and three builds proved it
The rule said a timed-out screenshot is a page mid-rerun and to re-navigate.
That is true and incomplete: it recovers a busy page, not a tab that has stopped
answering, and the timeout message is identical for both.
Three builds on three notebooks:
t1p re-navigated the same tab three times over ~40 seconds, every screenshot
still timing …
…s (#408) * Make main's lint and doc-link checks pass again pylint 4 reports a false positive E1136 on probe.py, where best_unmask is guarded by an 'is None' check. assert_doc_links_resolve flagged skill files cited by bare name, because it resolved paths only against the repo root and docs/; it now also tries .claude/skills/ and the citing file's directory. The two citations of eval/ files, which were never committed, are reworded to say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FZpJVZB4xzz8ULwYZV4p7C * Remove references to the uncommitted eval/ harness The eval/ directory is gitignored and never committed, so citations of its scripts and files point at nothing. Drop them from CLAUDE.md, .claude/language.json, ADR 0004 and the design doc, including the design doc's section on the self-improvement loop, which only described that tooling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6mRDBmoUnqfc83jkhS4sy * Leave the design docs as they are on main The design doc and ADRs are a historical record, so this PR no longer edits them. The doc-link check instead skips references to gitignored paths such as eval/, which are deliberately not committed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6mRDBmoUnqfc83jkhS4sy --------- Co-authored-by: Claude <noreply@anthropic.com>
…ld (#409) * ci(windows): get OpenMS from a reusable workflow instead of a contrib build Add .github/workflows/openms-windows.yml (workflow_call). Installer mode repackages bin/ and share/ of the official OpenMS Windows installer; source mode builds a branch or fork with the vcpkg preset windows-x64-release and CPack ZIP. Both upload the openms-package artifact build-executable already consumes, after a --help smoke test of the kept TOPP tools. build-windows-executable-app.yaml now calls it in installer mode for 3.5.0, dropping contrib, the Qt install and the cibuild/citest/cipackage steps. A temporary workflow exercises source mode against OpenMS develop on this branch; it is removed before review. Part of OpenMS/OpenMS#10327 (step C2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ALo85PSS7JaNtrk18Au5qs * Port #408: make main's lint and doc-link checks pass main (a9a3c7a) is red on Pylint (probe.py E1136) and on the assert-invariants doc-link check, independently of this PR. This is the change of OpenMS/streamlit-template#408, verbatim, so this PR's CI can go green; it no-ops once #408 is merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ALo85PSS7JaNtrk18Au5qs * Remove the temporary source-mode test; document measured timings Source mode ran against OpenMS develop on this branch: cold 2h50m (vcpkg dependencies 1h23m, OpenMS compile 1h24m), warm 8m12s. The temporary workflow that ran it is no longer needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ALo85PSS7JaNtrk18Au5qs * Address review: per-file doc-link resolution, no template expansion in run ci-assertions.sh: a reference that resolves only relative to its citing file's directory now has to resolve for every file citing it, not just for one; the failure names only the citing files that do not resolve it. This exposed three skill references that pointed at scaffold-workflow-app/ files from another skill's directory; they now name that directory. openms-windows.yml: the source ref and the release tag and asset names reach the run scripts through env instead of ${{ }} expansion, so a ref containing $(...) or backticks is never evaluated by bash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ALo85PSS7JaNtrk18Au5qs * Drop the Windows workflow docs; shorten its comments docs/openms-windows-workflow.md would ship in every app's Windows installer, since build-executable copies docs/. The description: fields on the workflow_call inputs document the interface. README's feature line goes back to main's wording, the 24-line header of openms-windows.yml is cut to four lines, and neither it nor the build-openms comment points at the removed doc any more. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NXe1BfF1D7H2YYQTKBN1GG --------- Co-authored-by: Claude <noreply@anthropic.com>
Brings in the template's TOPP workflow framework changes since the last sync (#95, which squashed the template up to 76a793e). Most importantly #397 (explicit flag parameters): run_topp no longer treats every value that reads "true"/"false" as a valueless on/off flag. FLASHApp dropped an explicit "false" for string parameters such as iq:isotope_correction (default "true"), so the -ini default silently won. FLASHDeconv's and FLASHTnT's real TOPP flags are now declared through input_TOPP(flag_parameters=...). Deployment files (.github/, k8s/, Dockerfiles, CLAUDE.md) keep FLASHApp's versions; the template's CI restructure and NFS storage stack are not adopted here. Template tests that exercise the template example workflow or its k8s/Dockerfile_simple infrastructure are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011bTAGp5Vwvv43y2daqT1cq
The template version documents its NFS storage stack, which this sync does not adopt for FLASHApp's k8s/. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011bTAGp5Vwvv43y2daqT1cq
polars 2.0 made pl.count() require a column, so the row count raised TypeError (test_render_compression fails on develop too). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011bTAGp5Vwvv43y2daqT1cq
Member
Author
|
Generated by Claude Code |
t0mdavid-m
marked this pull request as ready for review
October 19, 2026 06:58
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
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.
Requested by Tom · project thread
Before: if you set a true/false string parameter such as
iq:isotope_correction(defaulttrue) tofalse, FLASHApp left it off the FLASHDeconv command line, so the-inidefaulttruewon. Setting such a parameter totruewhen its default isfalse(e.g.use_RNA_averagine) produced a bare-namewith no value.After: every parameter goes out with its value (
-iq:isotope_correction false). Only the real TOPP switches (registerFlag_in the tool source) are sent as valueless flags.The cause was FLASHApp's own
run_topp: it treated any value that readstrue/falseas a valueless flag. The template replaced that guessing with explicitflag_parametersin OpenMS/streamlit-template#397. FLASHApp had not picked that up yet.How
main(3916a5b) intodevelop, as a real merge commit. The previous sync (Merge upstream OpenMS/streamlit-template into FLASHApp #95) was a squash, so git had no record of what was already merged. I resolved this merge against 76a793e, the template commit Merge upstream OpenMS/streamlit-template into FLASHApp #95 had synced to, and future syncs will find the right base on their own.src/Workflow.pydeclaresFLASHDECONV_FLAGS = ['keep_empty_out', 'write_detail']andFLASHTNT_FLAGS = ['only_single_hit', 'discard_underdetermined', 'keep_decoy'], taken from the FVdeploy tool sources. They're passed to all threeinput_TOPPcalls..github/,k8s/, the Dockerfiles,CLAUDE.md,content/quickstart.pyanddocs/kubernetes-deployment.md. The template's CI restructure and its NFS storage stack are not adopted. Its 400Gi workspace claim on a new storage class would have replaced FLASHApp's 3Ti claim. Because this is a real merge, those template changes count as merged, so adopting them later means porting them by hand.test_tasks.py,test_seed_demos.py,test_storage_health.py.ParameterManager/StreamlitUI/tasks/QueueManagerupdates,settings_io.py, thehealth.pyupdate, and the.claude/skills converted to directories along with the notebook-to-webapp skills and docs.Checked
pytest tests/: 163 passed, 1 failed. The failure,test_render_compression::test_nonempty_input_passes_through_binning, fails the same way ondevelop.pylint --errors-only: no errors in the changed files. The remaining errors are in untouched modules and also appear on develop._flag_params.run_toppthen buildsFLASHDeconv … -iq:type tmt6plex -iq:isotope_correction false -keep_empty_out -threads 1.Heads-up: an existing workspace whose
params.jsonhas no_flag_paramsgets it on the first visit to the Configure page. Until then, a flag that is switched on would be sent as-keep_empty_out True.🤖 Generated with Claude Code
https://claude.ai/code/session_011bTAGp5Vwvv43y2daqT1cq
Generated by Claude Code