Skip to content

style: backtick env vars in help prose; enforce the ADE_* rule in the style test - #188

Merged
lai-bianca merged 2 commits into
mainfrom
backtick-env-vars-help-prose
Aug 11, 2026
Merged

style: backtick env vars in help prose; enforce the ADE_* rule in the style test#188
lai-bianca merged 2 commits into
mainfrom
backtick-env-vars-help-prose

Conversation

@lai-bianca

Copy link
Copy Markdown
Collaborator

Follow-up to the post-merge review feedback on #166 (#166 (comment)): the env overrides convention entry was left unbackticked even though docs/agents/writing-style.md names environment variables as a backtick category, and the style test could not catch it because it only matched --flag shapes.

Changes

  • Backtick ADE_HOME, ADE_API_KEY, and ADE_ENDPOINT in the env overrides convention entry (the reviewed miss).
  • Fix the sibling misses in help-surface prose the same manual pass skipped: ADE_API_KEY in the auth status result key, ADE_HOME in the store home line, and five prose $ADE_ENV mentions normalized to `ADE_ENV` (the style doc's notation). The topic-body mention stays bare per the pre-formatted exemption.
  • Extend tests/test_help_style.py with a BARE_ENV_VAR check so the env-var rule is enforced the same way the flag rule is. The test's original rationale for flag-only scope was that a long flag is unmistakable in prose; an ADE_* env var is equally unmistakable. The pattern also flags $-prefixed mentions.
  • Add the previously unscanned store.home and store.note fields to the prose scan, which surfaced one more bare --json in the store note (now backticked).
  • Update docs/agents/writing-style.md to state that both rules are enforced and that env vars are written as a backticked bare name (ADE_ENV, not $ADE_ENV).
  • Regenerate the docs/reference/help.json snapshot.

Deliberately out of scope: bare env vars in error and status messages outside the help surface (for example the login hints in auth.py and credentials.py); those are not part of the help --json contract the hosted docs consume.

Full suite passes (743 passed, 8 skipped); uvx ruff check src tests and uvx ty check src are clean.

🤖 Generated with Claude Code

…le test

PR #166 backticked bare flags across the help surface but left
environment variables bare, which docs/agents/writing-style.md names as
a backtick category (post-merge review feedback on #166). Backtick
ADE_HOME / ADE_API_KEY / ADE_ENDPOINT in the env-overrides convention
entry plus the sibling misses ($ADE_ENV in flag help normalizes to
`ADE_ENV`), extend tests/test_help_style.py to enforce the env-var rule
the same way it enforces flags, scan the store home/note fields it
previously skipped (catching one bare --json), and regenerate the
help.json snapshot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lai-bianca
lai-bianca requested review from DanGH1 and yuanwen-tian and a balanced review from Copilot August 11, 2026 04:52

Copilot AI 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.

Pull request overview

Updates help prose styling and strengthens automated enforcement for ADE_* environment variables.

Changes:

  • Backticks environment variables and flags across help text.
  • Adds environment-variable style validation and expands scanned fields.
  • Updates writing guidance and regenerates the help snapshot.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_help_style.py Adds environment-variable checks and store fields.
src/ade_cli/parse.py Normalizes ADE_ENV help prose.
src/ade_cli/help.py Backticks help-surface literals.
src/ade_cli/extract.py Normalizes ADE_ENV help prose.
src/ade_cli/auth.py Normalizes authentication help prose.
docs/reference/help.json Regenerates the help reference.
docs/agents/writing-style.md Documents enforced environment-variable notation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_help_style.py
Comment thread tests/test_help_style.py
Copilot review on #188 found two guard gaps: topic titles are prose
rendered in help and `help --json` but were not scanned (only bodies
are exempt), and the code-span strip hid a backticked `$ADE_ENV` from
the env-var check, so the documented not-`$ADE_ENV` notation was
unenforced. Scan titles, and match $-prefixed mentions against the raw
text. Test-only: the help surface itself is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DanGH1

DanGH1 commented Aug 11, 2026

Copy link
Copy Markdown

Thanks for the quick follow-up, @lai-bianca — this fully resolves the env-var gap from #166#issuecomment-5248246441. I re-scanned every string in this PR's help.json (not just the fields test_help_style.py itself checks) and the only bare ADE_* mentions left are the two inside the credentials topic body ($ADE_ENV, export ADE_API_KEY=...), which are correctly covered by the pre-formatted exemption. Nice touch extending the style test itself so this category is now enforced, not just patched.

One new thing this PR surfaces, worth a look before merging: the store.home change —

"~/.ade (ADE_HOME overrides)"  ->  "~/.ade (`ADE_HOME` overrides)"

is the right instinct, but the docs repo's generate_cli_reference.py renders this field through a code_cell() helper that unconditionally wraps whatever it's given in another pair of backticks:

def code_cell(text):
    return "`" + text.replace("|", "\|") + "`"

I ran the resulting string through an actual CommonMark parser to check, rather than just eyeballing it:

Source: The local store lives at `~/.ade (`ADE_HOME` overrides)`.
Rendered: <p>The local store lives at <code>~/.ade (</code>ADE_HOME<code> overrides)</code>.</p>

ADE_HOME ends up as plain, unstyled text sandwiched between two mismatched code spans — worse than the original unbackticked version, not better. This isn't something ade-cli's own tests can catch (they have no visibility into the docs repo's generator), so it'd only show up the next time the CLI reference regenerates. Might be worth either not backticking ADE_HOME inside that specific field, or fixing code_cell() on the docs side to escape/handle embedded backticks before this next ships in a release.

@lai-bianca

Copy link
Copy Markdown
Collaborator Author

Thanks @DanGH1 — good catch on the code_cell() collision. This is now addressed on the docs side, in the still-open CLI docs PR (landing-ai/docs#377, commit landing-ai/docs@92e88eb):

  • store.home now renders through prose(), which preserves embedded code spans, since the field is mixed content rather than a bare path.
  • code_cell() additionally strips embedded backticks before wrapping, so any future mixed-content field degrades to a single clean code span instead of mismatched spans.

Verified with markdown-it against this PR's help.json:

Source:   The local store lives at ~/.ade (`ADE_HOME` overrides).
Rendered: <p>The local store lives at ~/.ade (<code>ADE_HOME</code> overrides).</p>

Since the fix lands in the docs PR before the generator ever ships, no change is needed in this PR, and the backticked store.home stays consistent with the style rule and its test.

🤖 Generated with Claude Code

@DanGH1 DanGH1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@lai-bianca
lai-bianca merged commit 2615a91 into main Aug 11, 2026
1 check passed
@lai-bianca
lai-bianca deleted the backtick-env-vars-help-prose branch August 11, 2026 22:57
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.

3 participants