Add feature flag system - #55
Merged
Merged
Conversation
Lightweight runtime feature flags for toggling behaviour via environment variables or programmatic assignment, with a scoped override context manager for tests. src/mobius/_flags.py: - Flags dataclass singleton; each field reads MOBIUS_<NAME> at import time - _env_bool(): case-insensitive bool from env var with fallback default - list_flags(): snapshot dict of all current flag values - override_flags(**kwargs): context manager that restores values on exit, even if an exception is raised (exception-safe) - Initial flag: suppress_dedup_warning (default True) - Docstring explains how to add new flags src/mobius/_flags_test.py: 23 tests covering defaults, all truthy/falsy env-var strings, unknown values, programmatic override, context manager (single, nested, exception safety), and list_flags() snapshot semantics. src/mobius/__init__.py: export flags and override_flags in public API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Performance Comparison
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a lightweight runtime feature-flag facility to mobius, intended to gate experimental/environment-specific behaviors via MOBIUS_* env vars or programmatic overrides.
Changes:
- Introduces
Flagsdataclass + globalflagssingleton,list_flags(), andoverride_flags()context manager. - Adds a new unit test module covering defaults, env var parsing, overrides, and
list_flags()behavior. - Exposes
flagsandoverride_flagsfrom the top-levelmobiuspackage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/mobius/_flags.py |
Implements the flag container, env parsing helper, global singleton, listing, and override context manager. |
src/mobius/_flags_test.py |
Adds pytest coverage for default behavior, env var parsing, and scoped overrides. |
src/mobius/__init__.py |
Re-exports flags and override_flags from the public package API. |
justinchuby
commented
Mar 28, 2026
justinchuby
commented
Mar 28, 2026
justinchuby
commented
Mar 28, 2026
1. Docstring accuracy: clarify env vars are read at Flags() construction time; the global singleton is constructed at import time (not import-time env var reading per se). 2. Per-field docstrings: added a string-literal docstring after the suppress_dedup_warning field for documentation generation. 3. Auto-generated flags table: class-level docstring now includes a list-table of all available flags with env var, default, and description. 4. Return type: list_flags() now returns dict[str, object] instead of dict[str, bool] (dataclasses.asdict returns dict[str, Any]; tighten to object for mypy strict compatibility). 5. Unknown-flag validation: override_flags() now raises ValueError with a helpful message listing available flags when an unknown key is passed. 6. Wire suppress_dedup_warning into _builder.py: _optimize() now only applies _suppress_dedup_empty_initializer_warnings() when the flag is True, making the flag actually functional. 7. Import style in tests: changed from individual symbol imports to module-level import (from mobius import _flags) so test code clearly shows where each symbol originates. 8. Unreachable code: replaced pytest.raises nested-with pattern with try/except in test_restored_on_exception; replaced double-with in test_unknown_flag_raises_value_error with single combined with statement (SIM117). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Thread safety note: - override_flags() docstring now explains it is not thread-safe (TOCTOU between save-old and restore-old), but safe for pytest -n auto because xdist workers are separate processes with independent flag singletons. Docs: - docs/feature-flags.md: covers available flags table, env var usage, programmatic override, override_flags() in tests (with thread-safety callout), list_flags(), and how to add new flags. - docs/index.md: added feature-flags to the toctree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Replace the manually-written docs page with an auto-generated one. - scripts/generate_flags_docs.py: introspects Flags via dataclasses.fields() and AST-parses _flags.py to extract env var names, defaults, and per-field docstrings from string literals after each field definition - docs/feature-flags.md: regenerated from the script - .github/workflows/check-flags-docs.yml: CI check that fails if the committed page is out of date with the Flags dataclass Developers add a new flag by adding a dataclass field + docstring in _flags.py, then running 'python scripts/generate_flags_docs.py'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
The flags module is internal-only. Users configure flags via environment variables (MOBIUS_*); only internal code imports from mobius._flags directly. - Remove flags, override_flags from __init__.py __all__ and imports - Update docs to use 'from mobius._flags import ...' instead of 'import mobius' - Regenerate docs/feature-flags.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
- Remove docs/feature-flags.md from repo (build-time artifact) - Remove check-flags-docs.yml workflow (no longer needed) - Add scripts/generate_flags_docs.py call to pages.yml Build Sphinx docs step - Gitignore docs/feature-flags.md to prevent accidental commits Follows the same pattern as generate_dashboard.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Move Sphinx documentation to the root URL (onnxruntime.github.io/mobius/) and the testing confidence dashboard to a sub-URL (onnxruntime.github.io/mobius/dashboard/). - Swap build order in pages.yml: sphinx-build to _site/, dashboard to _site/dashboard/ - Rename workflow from 'Dashboard' to 'Documentation' - Add dashboard link to docs index page - Add docs link to dashboard header Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
force-pushed
the
justinchu/feature-flags
branch
from
March 28, 2026 04:02
d10d64a to
dc3583b
Compare
Matches the convention of docs/_generate_models.py. Update pages.yml and self-references in the script. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
force-pushed
the
justinchu/feature-flags
branch
from
March 28, 2026 04:04
dc3583b to
8a5d428
Compare
Move model page generation, feature-flags doc generation, and dashboard generation into Sphinx extensions under docs/_ext/. This consolidates the entire documentation build into a single sphinx-build invocation: - models_gen: builder-inited hook runs _generate_models.py - flags_gen: builder-inited hook runs _generate_flags_docs.py (conditional) - dashboard: build-finished hook runs generate_dashboard.py into /dashboard/ and creates /docs/ → / redirect Simplify pages.yml from 4 separate steps to 1 sphinx-build command. Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
…justinchu/feature-flags Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> # Conflicts: # .github/workflows/pages.yml
- Replace removed app.warn() with logging.getLogger(__name__).warning() - Replace 'app.verbosity and print(...)' with logger.info() - Add debug log in flags_gen when generator script is absent Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Lightweight feature flag system for mobius. Flags are configured via environment variables (
MOBIUS_<NAME>) or programmatically. Includesoverride_flags()context manager for tests.Initial flag:
suppress_dedup_warning(default:True)Changes
src/mobius/_flags.py:Flagsdataclass singleton,list_flags(),override_flags()context managersrc/mobius/_flags_test.py: 23 tests (defaults, env vars, programmatic override, context manager, list_flags)