docs: finalize v7.61 deployment guide and architecture overhaul#20
Merged
Conversation
- Added deployment capacity and Docker benchmarks to README - Promoted SPEC.md status to Stable with operational characteristics - Added benchmarking instructions to CONTRIBUTING.md - Documented worker deployment scenarios in Examples/README.md - Added deployment security guidelines to SECURITY.md - Added Enterprise Stress Suite (stest_ui.py) to tests/ - Scrubbed legacy URLs from example worker scripts
Contributor
📝 WalkthroughWalkthroughThis PR marks the protocol stable for v7.61, adds deployment/performance/security documentation and example updates pointing to a Render host, extends contribution docs with stress-test instructions, and introduces a full stress-testing suite with live dashboarding and JSONL/JSON reporting. Changesv7.61 Deployment, Performance & Testing Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
README.md (1)
376-405: ⚡ Quick winAlign deployment guidance with architecture diagram to avoid mixed hosting signals.
Line 380+ positions Docker/Render as the validated deployment path, but the architecture diagram still labels PythonAnywhere. Please update the diagram host label so docs don’t present conflicting defaults.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 376 - 405, The architecture diagram label still shows "PythonAnywhere" which conflicts with the README's validated deployment recommendation of Docker/Render; update the architecture diagram graphic (the host label text in the diagram asset referenced by the README) to reflect the recommended deployment (e.g., "Docker/Render" or a neutral "Cloud Platform") so the diagram matches the Docker/docker-compose guidance, and ensure any alt text/caption in README.md that references the diagram is updated to the same host label.tests/stest_ui.py (2)
440-443: 💤 Low valueBroad
Exceptioncatches are intentional but could mask unexpected bugs.The bare
Exceptionhandlers (lines 440, 537) prevent worker crashes during stress tests, which is reasonable. However, consider logging the exception type in addition to the message to aid debugging if unexpected errors occur.♻️ Optional: Include exception type in log
except Exception as e: metrics.inc("err_unknown") - logger.log_event("publish_unknown_error", {"error": str(e)}) + logger.log_event("publish_unknown_error", {"error": str(e), "type": type(e).__name__}) time.sleep(0.5)Also applies to: 537-541
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/stest_ui.py` around lines 440 - 443, Update the broad except blocks that currently do metrics.inc("err_unknown") and call logger.log_event("publish_unknown_error", {"error": str(e)}) so the logged payload also includes the exception type; for example, augment the logger.log_event call to include type(e).__name__ (and optionally full module path) alongside the message, and apply this change to both except Exception as e handlers (the one around logger.log_event("publish_unknown_error") and the similar handler at lines ~537) so the logs contain both error message and exception type for easier debugging.
62-74: 💤 Low valueDefer API key loading to avoid module-level side effects.
Loading
API_KEYat module level (line 74) causesSystemExiton import if~/.apikeyis missing. This prevents importing the module for testing or tooling purposes. Consider lazy initialization or moving the call intorun_profile().♻️ Proposed refactor: Lazy initialization
-API_KEY = get_api_key() +API_KEY: str | None = None + +def _get_api_key() -> str: + global API_KEY + if API_KEY is None: + API_KEY = get_api_key() + return API_KEYThen update
IntentClientinstantiation in workers to use_get_api_key().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/stest_ui.py` around lines 62 - 74, The module currently calls get_api_key() at import time (API_KEY = get_api_key()) which raises SystemExit if ~/.apikey is missing; change this to defer loading by removing the module-level API_KEY and adding a lazy accessor (e.g. _get_api_key() or make get_api_key() only invoked at runtime), then update all call sites (for example where IntentClient is instantiated in workers and any use in run_profile()) to call the lazy accessor at runtime instead of importing API_KEY at module import time so importing the module no longer produces side effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 435: Fix the README entry for BUS_TRUST_PROXY: correct the typo and
improve formatting by changing the description text to read "For servers behind
proxies; set to 'true'." Make sure the variable name BUS_TRUST_PROXY remains
unchanged and update the casing and punctuation so it matches other entries in
the table.
In `@tests/stest_ui.py`:
- Around line 744-751: The -h short flag for the high-intensity profile
conflicts with argparse help; change the group.add_argument call that currently
uses "-h", "--high" to use a non-conflicting short option (e.g. "-H") and a
clearer long name (e.g. "--high-only" or "--high") so the signature becomes
something like "-H", "--high-only" and update any code that checks for args.high
accordingly; after switching the short flag you can also remove add_help=False
and the manual "--help" argument (if present) to restore argparse's default help
handling.
---
Nitpick comments:
In `@README.md`:
- Around line 376-405: The architecture diagram label still shows
"PythonAnywhere" which conflicts with the README's validated deployment
recommendation of Docker/Render; update the architecture diagram graphic (the
host label text in the diagram asset referenced by the README) to reflect the
recommended deployment (e.g., "Docker/Render" or a neutral "Cloud Platform") so
the diagram matches the Docker/docker-compose guidance, and ensure any alt
text/caption in README.md that references the diagram is updated to the same
host label.
In `@tests/stest_ui.py`:
- Around line 440-443: Update the broad except blocks that currently do
metrics.inc("err_unknown") and call logger.log_event("publish_unknown_error",
{"error": str(e)}) so the logged payload also includes the exception type; for
example, augment the logger.log_event call to include type(e).__name__ (and
optionally full module path) alongside the message, and apply this change to
both except Exception as e handlers (the one around
logger.log_event("publish_unknown_error") and the similar handler at lines ~537)
so the logs contain both error message and exception type for easier debugging.
- Around line 62-74: The module currently calls get_api_key() at import time
(API_KEY = get_api_key()) which raises SystemExit if ~/.apikey is missing;
change this to defer loading by removing the module-level API_KEY and adding a
lazy accessor (e.g. _get_api_key() or make get_api_key() only invoked at
runtime), then update all call sites (for example where IntentClient is
instantiated in workers and any use in run_profile()) to call the lazy accessor
at runtime instead of importing API_KEY at module import time so importing the
module no longer produces side effects.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6034c6a5-18b6-45d9-8af9-eee57ad9a172
📒 Files selected for processing (10)
CHANGELOG.mdCONTRIBUTING.mdExamples/README.mdExamples/discord_worker.shREADME.mdSECURITY.mdSPEC.mdlogger.shtests/stest_ui.pyworker.sh
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
435-436:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPolish
BUS_TRUST_PROXYrow text and table spacing.The description still has inconsistent casing, and the table block likely still violates markdownlint spacing before the horizontal rule.
Suggested doc fix
-| `BUS_TRUST_PROXY` | — | For Servers behind proxies; set to 'true'| +| `BUS_TRUST_PROXY` | — | For servers behind proxies; set to 'true'. | + ---🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 435 - 436, Update the BUS_TRUST_PROXY table row to use consistent sentence casing and fix markdown table spacing before the horizontal rule: change the description cell for the `BUS_TRUST_PROXY` row to "For servers behind proxies; set to 'true'." (sentence case) and ensure there is a blank line before the table separator line (the --- row) so the table complies with markdownlint spacing rules; locate and edit the table row containing `BUS_TRUST_PROXY` to apply these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@README.md`:
- Around line 435-436: Update the BUS_TRUST_PROXY table row to use consistent
sentence casing and fix markdown table spacing before the horizontal rule:
change the description cell for the `BUS_TRUST_PROXY` row to "For servers behind
proxies; set to 'true'." (sentence case) and ensure there is a blank line before
the table separator line (the --- row) so the table complies with markdownlint
spacing rules; locate and edit the table row containing `BUS_TRUST_PROXY` to
apply these changes.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/stest_ui.py (1)
62-74: ⚡ Quick winModule-level API key loading blocks import without
~/.apikey.Executing
get_api_key()at import time means the module cannot be imported for testing, documentation, or any other purpose without~/.apikeypresent. Consider lazy initialization or environment variable fallback.♻️ Suggested refactor: lazy initialization
-API_KEY = get_api_key() +_API_KEY: str | None = None + +def get_api_key_cached() -> str: + global _API_KEY + if _API_KEY is None: + _API_KEY = get_api_key() + return _API_KEYThen update usages in
publisher_workerandruntime_worker:- client = IntentClient(base_url=BASE_URL, api_key=API_KEY, timeout=30) + client = IntentClient(base_url=BASE_URL, api_key=get_api_key_cached(), timeout=30)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/stest_ui.py` around lines 62 - 74, The module currently calls get_api_key() at import-time which raises SystemExit if ~/.apikey is missing, blocking imports; change to lazy initialization and fallback to an environment variable: update get_api_key to return None or raise only when explicitly requested, add a new accessor function (e.g., fetch_api_key() or get_api_key_or_raise()) that checks os.environ["API_KEY"] before reading Path.home()/".apikey" and only raises SystemExit when called by runtime entrypoints; replace the module-level API_KEY constant with a lazily-evaluated lookup used in publisher_worker and runtime_worker so tests/docs can import the module without the file present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/stest_ui.py`:
- Around line 62-74: The module currently calls get_api_key() at import-time
which raises SystemExit if ~/.apikey is missing, blocking imports; change to
lazy initialization and fallback to an environment variable: update get_api_key
to return None or raise only when explicitly requested, add a new accessor
function (e.g., fetch_api_key() or get_api_key_or_raise()) that checks
os.environ["API_KEY"] before reading Path.home()/".apikey" and only raises
SystemExit when called by runtime entrypoints; replace the module-level API_KEY
constant with a lazily-evaluated lookup used in publisher_worker and
runtime_worker so tests/docs can import the module without the file present.
This was referenced May 25, 2026
Merged
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores