Skip to content

docs(readme): every environment variable the package reads is documented, and a grader says so - #3313

Merged
cagataycali merged 3 commits into
strands-labs:mainfrom
cagataycali:env-vars-read-are-documented
Sep 8, 2026
Merged

cagataycali merged 3 commits into
strands-labs:mainfrom
cagataycali:env-vars-read-are-documented

Conversation

@cagataycali

@cagataycali cagataycali commented Sep 8, 2026

Copy link
Copy Markdown
Member

What

The README's Configuration table calls itself the single source of truth for STRANDS_* variables, and AGENTS.md asks that a new one land there in the same change that introduces it. Nothing graded the population. The reference pages were checked one variable set at a time, each by a test written for the change that added it (tests/mesh/test_docs_*_env_var_reference.py), so a variable introduced without such a test was undocumented by default - and the omission was silent in the reassuring direction: the code honoured it, the tests that setenv it passed, and no page a reader could reach named it.

Measured

AST walk over strands_robots/ at 9770194: 88 distinct STRANDS_* names across 107 sites. 59 are read with os.getenv / os.environ.get / .setdefault / [...] at the call site; 29 reach the environment only through a resolver the package wrote for itself - _int_env, _float_env, _bool_env, _resolve_hz, hz_from_env, _env_switch, resolve_sandbox_root and six more. Matched as whole tokens against README.md + docs/**/*.md, honouring the README's sibling shorthand (`STRANDS_MESH_POSE_HZ`, `_IMU_HZ`):

variable read at how
STRANDS_MESH_CAMERA_S3_BUCKET mesh/iot/camera_offload.py:174 direct
STRANDS_MESH_CAMERA_S3_PREFIX mesh/iot/camera_offload.py:175 direct
STRANDS_GR00T_REPO_URL tools/gr00t_inference.py:222 direct
STRANDS_GR00T_REPO_TAG tools/gr00t_inference.py:227 direct
STRANDS_MESH_BRIDGE_DEDUP_STRICT mesh/transport/bridge_transport.py:314 direct
STRANDS_MESH_FILTER_INTERFACES mesh/_zenoh_config.py:472 direct
STRANDS_ROBOTS_VERBOSE_MUJOCO simulation/mujoco/backend.py:697 direct
STRANDS_MESH_MAX_CMD_BYTES mesh/_zenoh_config.py:210, :501 _int_env
STRANDS_MESH_MAX_CAMERA_BYTES mesh/_zenoh_config.py:507 _int_env
STRANDS_MESH_MAX_SAFETY_BYTES mesh/_zenoh_config.py:513 _int_env
STRANDS_MESH_MAX_SESSIONS mesh/_zenoh_config.py:390 _int_env
STRANDS_MESH_CMD_RATE_HZ mesh/_zenoh_config.py:407 _float_env
STRANDS_MESH_SAFETY_RATE_HZ mesh/_zenoh_config.py:418 _float_env
STRANDS_MESH_CAMERA_DISABLED mesh/core.py:1572 _bool_env as _zc_bool_env
STRANDS_ISAAC_CAMERA_WARMUP_STEPS simulation/isaac/simulation.py:928 _env_int
the other 73 documented (README, or docs/security.md for the IoT credentials and TLS material)

15 undocumented anywhere. Two of the direct ones sit next to a documented sibling that only makes sense once the missing one is set: STRANDS_MESH_CAMERA_PRESIGN_TTL is in the README and the bucket that turns the offload on is not, so the table described a knob on a feature it gave no way to enable; STRANDS_GR00T_REPO_URL_ALLOW is in docs/security.md with no mention of the variable it constrains. Of the resolver-read ones, the six mesh transport caps are the DoS bounds docs/security.md describes in prose without naming the knobs, and STRANDS_MESH_CAMERA_DISABLED is a privacy kill switch that raised on a bad spelling precisely so it could not be silently mis-set - and could not be found.

A regex over string literals finds 62 direct names, not 59. The three extra (STRANDS_SAFETY_TABLE, STRANDS_ESTOP_DEDUP_TTL_S, one more) live inside the e-stop fan-out Lambda's source, which mesh.iot.bootstrap ships as text and whose environment it provisions itself. Those are not reads this process makes, which is why the grader walks the AST rather than the text.

Change

README.md (+16 rows, no existing row edited): each missing variable gets a row beside the sibling it belongs with, worded from its read site - the accepted spellings, the bounds the resolver enforces, what the value selects, and the default. STRANDS_GR00T_REPO_URL_ALLOW is added to the README group too so the three clone knobs read as one (it stays in docs/security.md).

tests/test_env_vars_the_package_reads_are_documented.py derives the population from the package and the documented set from the pages, so a variable added later is graded on arrival. The resolver set is derived too: a function is a resolver when its body reads the environment - directly, or through a resolver already found - with a key that is one of its own positional parameters, iterated to a fixed point (so hz_from_env is found through _float_env) and followed through import aliases. Boundary cells: a resolver, a delegating resolver, an alias, and the negatives - passing a name into a refusal's wording is not a read, a name inside a string literal is not a read, a longer name does not document its prefix (..._URL_ALLOW is not ..._URL), a bare _IMU_HZ proves nothing without a documented sibling on the same page. Floors on the population (60) and page count (3) refuse a walk that reads nothing. Names outside the STRANDS_ prefix (MUJOCO_GL, ZENOH_CONNECT, GROOT_API_TOKEN) are another tool's to document and are out of scope.

Composition with the open set

This grader reads files it never names, so check_merge_base_overlap.py --all-open cannot see it (the class #2561 records). Checked by hand against the three open sibling diffs: #3212 adds STRANDS_MESH_PEER_RETENTION_S and documents it in README + docs/mesh.md (clean); #3205 adds STRANDS_DASHBOARD_PEER_TTL_S (documented in docs/dashboard/troubleshooting.md) and STRANDS_DASHBOARD_POSE_HZ through _env_float - that one appears in no .md hunk of that branch, so if #3205 lands after this it will need one row (its _env_float is a resolver by this grader's rule). #2907 adds no env read. #3312 not composed (opened in parallel; a fix(policies/vera) docker-query bound, no env read expected).

Verification

  • Grader on the pre-change README: 1 failed naming exactly the fifteen above; after: 18 passed.
  • scripts/check_whole_tree_graders.py after the README edit: 115 graders derived from the tree including this one, 4713 passed, 52 skipped.
  • ruff check / ruff format --check / mypy clean on the new file; no non-ASCII in it.

Round changelog

  • R0: opened as draft; fragment pushed; then re-measured before marking ready and found the first cut recognised only direct reads (59 names) - widened to resolver reads (88 names), which surfaced eight more undocumented variables; rows and cells added in 0cca008e. Marked ready.

…ted, and a grader says so

The README's Configuration table calls itself the single source of truth for
STRANDS_* variables and AGENTS.md asks that a new one land there in the same
change, but nothing graded the population: the reference pages were checked
one variable set at a time by tests written for the change that added each,
so a variable introduced without one was undocumented by default.

Measured on main, the package reads 59 distinct STRANDS_* names and seven
appear on no page under README.md or docs/:

  STRANDS_MESH_CAMERA_S3_BUCKET / _PREFIX  - the two that turn the camera S3
      offload on; the TTL that only matters once it is on was documented
  STRANDS_GR00T_REPO_URL / _TAG            - the clone source build_image
      fails closed on; its allowlist was documented without the variable
      it constrains
  STRANDS_MESH_BRIDGE_DEDUP_STRICT
  STRANDS_MESH_FILTER_INTERFACES
  STRANDS_ROBOTS_VERBOSE_MUJOCO

Each gets a row beside the sibling it belongs with, worded from the read
site. STRANDS_GR00T_REPO_URL_ALLOW is added to the README row group too so
the three clone knobs read as one.

tests/test_env_vars_the_package_reads_are_documented.py derives the
population from the package by AST (os.getenv, os.environ.get / setdefault /
subscript, and the bare imported spellings) and the documented set from
README.md plus docs/**/*.md, honouring the README's sibling shorthand
(`STRANDS_MESH_POSE_HZ`, `_IMU_HZ`) only where a documented full name shares
the prefix. A name inside a string literal is not a read, which is what keeps
the Lambda source mesh.iot.bootstrap ships out of scope by construction.
Floors on the population and page count refuse a walk that reads nothing.
Fails on the pre-change README naming all seven; 14 passed after.
…ows follow

Measured before marking ready: the package reads 88 STRANDS_* names, and 29
of them reach the environment only through a resolver - _int_env,
_float_env, _bool_env, _resolve_hz, hz_from_env, _env_switch and friends -
rather than through os.getenv at the call site. The first cut recognised the
four direct spellings alone, saw 59 names, and reported a clean tree that
was not one: eight of the 29 were undocumented, six of them the mesh
transport caps (_MAX_CMD_BYTES, _MAX_CAMERA_BYTES, _MAX_SAFETY_BYTES,
_CMD_RATE_HZ, _SAFETY_RATE_HZ, _MAX_SESSIONS), plus STRANDS_MESH_CAMERA_DISABLED
(reached only through `_bool_env as _zc_bool_env`) and
STRANDS_ISAAC_CAMERA_WARMUP_STEPS.

The grader now derives the resolver set from the tree - a function is one
when its body reads the environment, directly or through a resolver already
found, with a key that is one of its own positional parameters - iterated to
a fixed point and followed through import aliases. Cells pin each shape: a
resolver, a resolver delegating to another, an alias, and the negative that
passing a name into a refusal's wording is not a read. README gains the
eight rows; the fragment and docstring are re-measured (88 / 59 / 29).
@cagataycali
cagataycali marked this pull request as ready for review September 8, 2026 01:53

@yinsong1986 yinsong1986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Adds 16 rows to the README's environment-variable tables for the fifteen STRANDS_* names the package reads but no page documented (plus STRANDS_GR00T_REPO_URL_ALLOW promoted into the README group), and a whole-tree grader (tests/test_env_vars_the_package_reads_are_documented.py) that derives the read population from the package by AST -- direct reads plus reads through the package's own resolvers, found to a fixed point and followed through import aliases -- and the documented set from README.md + docs/**/*.md, so the README rule AGENTS.md states finally has an enforcer. Verified independently against the head tree: the walk finds 88 names, all documented, and all 18 tests in the new module pass; the grader run against the pre-change README fails naming exactly the missing set. Spot-checked the new rows against their read sites (camera_offload.py:174, gr00t_inference.py:222, mesh/core.py:1572, isaac/simulation.py) and the descriptions match the code's accepted spellings, bounds and defaults. No existing row is edited, no env var is added or renamed, so there is no one-way-door surface here. The test module is pure ASCII, carries no host paths, names its classes for behaviour, derives its walk root from strands_robots.__file__ rather than a path literal, and its floors (60 names / 3 pages) guard against the walk silently reading nothing.

What's good

  • The population and the resolver set are both derived from the tree, so a variable or resolver added later is graded on arrival with no edit to the test.
  • The negative cells (a name in a refusal's wording, a name inside shipped Lambda source, a longer name not documenting its prefix, a bare shorthand with no sibling) pin exactly the false-positive/false-negative directions this kind of grader drifts in.
  • The PR description's composition-with-open-set section does by hand what check_merge_base_overlap.py cannot see for this grader class (#2561), and names the one open sibling (#3205) that will owe a row if it lands after this.

Verification suggestions

  • After #3205 (or any branch adding an env read) lands, this grader is the composition check itself: pytest tests/test_env_vars_the_package_reads_are_documented.py -q on the merged tree.

@cagataycali
cagataycali enabled auto-merge (squash) September 8, 2026 02:23
@cagataycali
cagataycali merged commit 69440de into strands-labs:main Sep 8, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants