Skip to content

fix: replace remaining runtime asserts with explicit ValueError checks#260

Open
nightcityblade wants to merge 2 commits into
Tanzania-AI-Community:developmentfrom
nightcityblade:fix/issue-249
Open

fix: replace remaining runtime asserts with explicit ValueError checks#260
nightcityblade wants to merge 2 commits into
Tanzania-AI-Community:developmentfrom
nightcityblade:fix/issue-249

Conversation

@nightcityblade

Copy link
Copy Markdown

Fixes #249 (partial — config.py)

Replaces all 7 assert statements in app/config.py:initialize_settings() with explicit if not ...: raise ValueError(...) checks.

Why this matters: These asserts validate required environment variables (Meta API keys, WhatsApp tokens, database URL) at startup. Under python -O, assert statements are stripped entirely, meaning the app would start with missing credentials and fail later with confusing errors.

What changed:

  • 7 assert (condition), "message"if not (condition): raise ValueError("message")
  • No behavior change under normal execution
  • Same error messages preserved

The remaining ~25 asserts in service files can be addressed in a follow-up PR.

@Ben-Temming

Copy link
Copy Markdown
Collaborator

I dont think it makes sense to do #249 partially, it is cleaner if we have a single PR that fixes it fully.

@alvaro-mazcu

Copy link
Copy Markdown
Member

Precisely the asserts located in the config.py do not suffer that much from the runtime, because Twiga is not up before the settings checks @nightcityblade

@nightcityblade

Copy link
Copy Markdown
Author

That's a fair point — since the config checks run at startup before the app is live, assert is effectively equivalent here. Thanks for explaining the context! Happy to close this if you'd prefer to keep the current approach.

@alvaro-mazcu

Copy link
Copy Markdown
Member

just reset your last commit and continue with the fix, please

@alvaro-mazcu

Copy link
Copy Markdown
Member

@nightcityblade what's the status of this?

@nightcityblade

Copy link
Copy Markdown
Author

Thanks for the nudge. I reset the earlier partial direction and expanded the fix to cover the remaining runtime assert checks in the app/service/tool paths, while keeping the existing config.py change in place.

What I updated in the amended commit:

  • replaced runtime assert checks with explicit ValueError branches in app/services/* and the tool handlers
  • kept the same validation intent, but without relying on optimization-sensitive assertions
  • re-ran the relevant service tests locally

Tests run:

  • META_API_VERSION=v1 META_APP_ID=test META_APP_SECRET=test WHATSAPP_CLOUD_NUMBER_ID=test WHATSAPP_VERIFY_TOKEN=test WHATSAPP_API_TOKEN=test DATABASE_URL=postgresql://user:pass@localhost:5432/twiga_test .venv/bin/python -m pytest tests/services/test_onboarding_service.py tests/services/test_state_service.py tests/services/test_messaging_service.py -q

@alvaro-mazcu alvaro-mazcu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remember to change the PR name after the changes, @nightcityblade

Comment thread app/config.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rollback the changes in this file, as they are executed before the app is running

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nightcityblade Remember this ☝️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nightcityblade nightcityblade changed the title fix: replace assert with ValueError in config validation fix: replace runtime asserts with explicit ValueError checks Apr 23, 2026
@alvaro-mazcu

Copy link
Copy Markdown
Member

@nightcityblade Check also the pre-commit error

@nightcityblade

Copy link
Copy Markdown
Author

Good catch, thanks. I pushed the Black formatting changes from the failing pre-commit run, so the hook should be green on the next pass.

@nightcityblade nightcityblade changed the title fix: replace runtime asserts with explicit ValueError checks fix: replace remaining runtime asserts with explicit ValueError checks Apr 28, 2026
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.

Replace Runtime assert Statements with Controlled Checks and Exceptions

3 participants