Skip to content

Normalize GitHub account casing#116

Merged
ramimbo merged 1 commit into
ramimbo:mainfrom
Ckeplinger199:codex/issue-103-github-account-casing
May 24, 2026
Merged

Normalize GitHub account casing#116
ramimbo merged 1 commit into
ramimbo:mainfrom
Ckeplinger199:codex/issue-103-github-account-casing

Conversation

@Ckeplinger199
Copy link
Copy Markdown

Bounty #103

Summary

  • normalize github:<login> account lookups to lowercase at the shared account boundary
  • keep API account pages, public account pages, and MCP get_balance aligned with payout account behavior
  • add regression coverage for a mixed-case GitHub account with existing ledger activity

Repro

Live account lookup currently treats mixed-case GitHub logins as separate empty accounts:

curl -s 'https://mrwk.ltclab.site/api/v1/accounts/github:ckeplinger199'
# exists=true, balance_mrwk=2455

curl -s 'https://mrwk.ltclab.site/api/v1/accounts/github:Ckeplinger199'
# exists=false, balance_mrwk=0

MCP get_balance shows the same split between github:ckeplinger199 and github:Ckeplinger199.

Validation

  • uv run --extra dev python -m pytest tests/test_api_mcp.py::test_github_account_views_normalize_mixed_case_logins -q -> 1 passed
  • uv run --extra dev python -m pytest tests/test_api_mcp.py tests/test_wallet_api.py tests/test_webhooks.py -q -> 58 passed, 1 warning
  • uv run --extra dev python -m pytest -q -> 122 passed, 2 warnings
  • uv run --extra dev ruff format --check app/main.py tests/test_api_mcp.py
  • uv run --extra dev ruff check app/main.py tests/test_api_mcp.py
  • uv run --extra dev python -m mypy app
  • git diff --check

@EShener
Copy link
Copy Markdown
Contributor

EShener commented May 24, 2026

No blockers from my review.

Evidence checked:

  • Inspected app/main.py: _normalized_account() now lowercases github:<login> at the shared account boundary used by /api/v1/accounts/{account}, /accounts/{account}, and MCP get_balance. This keeps the change focused and does not alter the existing mrwk1 normalization path.
  • Confirmed the live production repro before this patch: curl -s https://api.mrwk.ltclab.site/api/v1/accounts/github:eshener returns exists=true and balance_mrwk=50, while curl -s https://api.mrwk.ltclab.site/api/v1/accounts/github:EShener returns exists=false and balance_mrwk=0; /accounts/github:EShener also renders the empty account state.
  • Ran the focused regression on PR head 7cf07bc: UV_CACHE_DIR=/private/tmp/uv-cache-mergework uv run --extra dev python -m pytest tests/test_api_mcp.py::test_github_account_views_normalize_mixed_case_logins -q -> 1 passed.
  • Ran related API/MCP/wallet/webhook coverage: UV_CACHE_DIR=/private/tmp/uv-cache-mergework uv run --extra dev python -m pytest tests/test_api_mcp.py tests/test_wallet_api.py tests/test_webhooks.py -q -> 58 passed, 1 warning.
  • Checked GitHub Actions: Quality, readiness, docs, and image checks is passing.

I do not see a blocker for this patch.

@Aspire-College
Copy link
Copy Markdown

No blockers from my pass.

Reviewed app/main.py and tests/test_api_mcp.py. The implementation is narrowly scoped to _normalized_account: MRWK wallet-address normalization remains unchanged, lowercase github: accounts now canonicalize the login portion, and other account namespaces still pass through. The regression exercises the three affected lookup surfaces:

  • /api/v1/accounts/github:Alice returns canonical github:alice, github_login=alice, and the existing ledger balance.
  • /accounts/github:Alice renders the normalized account balance and proof link.
  • MCP get_balance returns the canonical github:alice: 50 MRWK result.

Verification run:

  • uv --native-tls run --extra dev python -m pytest tests/test_api_mcp.py::test_github_account_views_normalize_mixed_case_logins -q -> 1 passed.
  • .\.venv\Scripts\ruff.exe format --check app/main.py tests/test_api_mcp.py -> 2 files already formatted.
  • .\.venv\Scripts\ruff.exe check app/main.py tests/test_api_mcp.py -> All checks passed.
  • git diff --check origin/main...HEAD -> passed.

Remaining risk: I did not rerun the full suite in this environment; the broader pytest/mypy commands could not launch through the uv-created Python shim inside this Windows sandbox. Given the tiny diff and passing focused regression/style checks, I do not see a blocker.

@ramimbo ramimbo merged commit 324aaa1 into ramimbo:main May 24, 2026
1 check passed
@ramimbo ramimbo added mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded labels May 24, 2026
Copy link
Copy Markdown

@weilixiong weilixiong left a comment

Choose a reason for hiding this comment

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

✅ No-blockers review — LGTM

Files: app/main.py, tests/test_api_mcp.py

Verified:

  • pytest test_api_mcp.py::test_github_account_views_normalize_mixed_case_logins: PASSED
  • pytest tests/ -x -q: 122 passed in 13.39s
  • ruff check: All checks passed

Adds lowercase normalization for GitHub:account links alongside mrwk1 address normalization. Clean implementation with comprehensive test coverage (2 views + MCP endpoint).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants