Skip to content

feat: Institutional framework foundation with TDD and GOTCHA comments#58

Open
StreetLevelTech1 wants to merge 1 commit into
devfrom
feat/institutional-framework-tdd
Open

feat: Institutional framework foundation with TDD and GOTCHA comments#58
StreetLevelTech1 wants to merge 1 commit into
devfrom
feat/institutional-framework-tdd

Conversation

@StreetLevelTech1
Copy link
Copy Markdown
Owner

What

Establishes the institutional-grade engineering foundation for StrideBot:

Documentation

  • ADR-0001: Adopt institutional engineering framework (MADR template)
  • Git Workflow Guide: Trunk-based development, conventional commits, PR process
  • SemVer Standard: Version 2.0.0 rules, bumping process, examples
  • Institutional Framework Spec: Full specification with risk assessment and governance

GOTCHA Comments (5 source files)

Added inline block comments documenting hidden gotchas:

  • : Blocking HTTP in async context, unbounded cache, CoinGecko rate limits
  • : Thread-unsafe globals, dual state (memory + DB)
  • : Unbounded semaphores, thread pool sizing, module-level executor
  • : Single-threaded HTTPServer, unbounded auth dict, no CSRF
  • : datetime.utcnow() deprecation, in-memory rate limit reset

Test Suite (5 new files, ~60 tests)

TDD approach with error-boundary tracking:

  • : Rate limits, quotas, admin bypass, history limits
  • : Mode JSON detection, semaphores, chunking, HTML fallback
  • : Auth, rate limiting, caching, CORS, config
  • : Coin resolution, batch refresh, 429 handling, cache growth
  • : Payload parsing, price validation, rate limiting, deprecation

Improved Tests

  • : Added 7 edge case tests for DB errors, concurrent access, empty memory

Why

StrideBot has grown from a solo project into a production system. The current workflow lacks:

  • Branch protection and PR review requirements
  • Versioning scheme (pyproject.toml shows 0.1.0 since inception)
  • Architectural decision documentation
  • Test coverage for critical paths (rate limiter, streaming, health dashboard)

This foundation establishes the governance needed for team collaboration and audit trails.

Testing

  • All 198 tests pass (2 skipped)
  • New tests follow TDD: write failing test first, then implement
  • GOTCHA comments document hidden gotchas and architectural dependencies

Checklist

  • Tests pass: uv run pytest bot/tests/ -v
  • No secrets in diff
  • Commit messages follow conventional format
  • Documentation updated (new guides and ADRs)

- Add docs/adr/ directory with MADR template and ADR-0001
- Add docs/guides/ with git-workflow.md and semver-standard.md
- Add GOTCHA comments to 5 source files documenting hidden gotchas:
  - data/prices.py: blocking HTTP, unbounded cache, CoinGecko rate limits
  - handlers/rate_limiter.py: thread-unsafe globals, dual state
  - handlers/streaming.py: unbounded semaphores, thread pool sizing
  - health.py: single-threaded HTTPServer, unbounded auth dict
  - handlers/mini_app.py: datetime.utcnow() deprecation, in-memory reset
- Add 5 new test files (~60 tests) with TDD approach:
  - test_rate_limiter.py: rate limits, quotas, admin bypass
  - test_streaming.py: mode detection, semaphores, chunking
  - test_health.py: auth, caching, CORS, config
  - test_prices.py: coin resolution, batch refresh, cache growth
  - test_mini_app.py: payload parsing, price validation, rate limiting
- Improve test_memory_functions.py with edge cases and error boundaries
- Add institutional framework spec at docs/specs/institutional-framework.md
- Add code review report at review/review2.md

All 198 tests pass (2 skipped).
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d0c4e5780

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bot/tests/test_prices.py
def test_known_mapping_returns_immediately(self):
"""Known coins resolve without API call."""
import data.prices as prices
assert prices.resolve_coin_id("btc") == "bitcoin"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Mock the CoinGecko refresh in cache-hit tests

In a fresh test process this call still performs a real CoinGecko /coins/list request, because resolve_coin_id() calls refresh_coin_cache_from_list() before checking _COIN_CACHE unless _COIN_CACHE_LAST_REFRESH is already within the TTL. That makes this unit test depend on network availability and can add 10-second stalls or failures in offline/429 environments; set _COIN_CACHE_LAST_REFRESH or monkeypatch prices.requests.get for the cache-hit/cooldown cases.

Useful? React with 👍 / 👎.

Comment thread bot/tests/test_health.py
Comment on lines +26 to +27
if health._DASHBOARD_DISABLED:
pytest.skip("Dashboard already disabled at import time")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Assert the disabled-dashboard auth path

When this condition is false the test falls through with no assertion or handler call, and when it is true it skips instead of verifying the 503 response. That means a regression in the production missing-secret guard for /api/stats or /api/health/all would still leave this auth test green; instantiate the handler or factor the auth check so this test exercises the disabled-dashboard response directly.

Useful? React with 👍 / 👎.

Comment on lines +101 to +102
rl.ai_usage_store[uid] = deque([now - i for i in range(20)])
assert rl.check_ai_quota(uid) is False
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Update the AI quota test to cover DB enforcement

This test seeds only ai_usage_store, but check_ai_quota() never appends to that deque after a successful request and relies on db.upsert_rate_limit() for the real per-hour counter. As a result, twenty normal calls with a DB mock that keeps returning True would all pass, so this test can give false confidence about the 20/hour quota; make the mock upsert_rate_limit count calls and return False on the 21st request instead of preloading unused in-memory state.

Useful? React with 👍 / 👎.

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.

1 participant