[aiwithrajan]

Description

Validates and sanitizes person names in CreatePerson and update_person_name.
Currently:

1. CreatePerson in backend/models/other.py defines name: str = Field(min_length=2, max_length=40) without stripping whitespace, permitting whitespace-only strings like " " to pass validation. It also creates entries with unstripped surrounding whitespace, preventing idempotent duplicate matching in get_person_by_name.
2. update_person_name in backend/routers/users.py receives a raw value: str (with the intended validation previously left commented out: # = Field(min_length=2, max_length=40)). When given a blank or whitespace-only string, the underlying rename transaction returns False, causing the endpoint to mistakenly raise HTTP 404 ("Person not found") instead of HTTP 422 Unprocessable Entity.

This PR:

• Adds a @field_validator('name') to CreatePerson to strip whitespace and enforce that stripped names are between 2 and 40 characters.
• Validates that value in update_person_name is non-blank with stripped length between 2 and 40 characters, raising HTTP 422 if invalid, and passes the clean name to update_person.
• Adds hermetic unit tests in backend/tests/unit/test_person_name_validation.py and router tests in backend/tests/routers/test_users.py.

Test Plan

• Run unit tests:

  PYTHONPATH=backend python3 backend/tests/unit/test_person_name_validation.py

  Result: 9/9 tests passed in 0.003s.

[Git-on-my-level]

Thanks @aiwithrajan - verified this end to end and it's a real bug fix.

Verified the premise against current main: in backend/database/person_aliases.py, a blank or whitespace-only name makes normalized_person_alias return None, update_person_name_transaction then returns False, and update_person_name raised 404 "Person not found" for what is a validation error. The new explicit 422 checks in backend/routers/users.py fix that misclassification, and passing clean_name into update_person means the stored name no longer depends on the alias layer to strip. Note this also intentionally tightens behavior: names of 41+ chars that previously slipped through (the alias layer allows up to 128) now get 422, consistent with the model's intended max_length=40.

backend/models/other.py: the field_validator on CreatePerson closes the real gap where " " passed min_length=2, and returns the stripped value so get_person_by_name matching is idempotent. field_validator is already imported on main, so no import change was needed. One nit, non-blocking: the 2-40 rule now lives in both the model validator and the router; a small shared helper would prevent future drift. Also the validator strips but does not collapse internal runs of whitespace, unlike normalized_person_alias's ' '.join(split()) - harmless today, just worth knowing the two normalizations differ slightly.

backend/tests/unit/test_person_name_validation.py: good hermetic coverage (valid, strip, boundaries 2/40, empty, whitespace-only, single char, 41 chars, 41 chars with padding). backend/tests/routers/test_users.py: the strip/blank/too-short/too-long router tests assert both the 422 and that update_person is not called on invalid input - exactly the right assertions.

Ran the new suites plus the full backend/tests/routers/test_users.py in a clean container on this head: 109 passed, 6 subtests passed.

No blocking concerns from me; leaving formal approval and merge to a human maintainer. This comment is AI-generated feedback on behalf of the maintainers.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[Git-on-my-level]

Thanks @aiwithrajan — verified this on head 6142487 in an isolated sandbox; all observations below are from actually running the code.

• backend/models/other.py — the new @field_validator('name') on CreatePerson mirrors the existing time_zone validators in the same file (strip -> bounds check -> return stripped), and the raw-length Field(min_length=2, max_length=40) stays as a first gate. field_validator was already imported at head, so no import-path risk. 9 unit tests + 6 subtests pass.
• backend/routers/users.py — update_person_name now strips and bounds-checks before calling update_person, so blank/whitespace renames get a correct 422 instead of the previous misleading 404 "Person not found". The stripped-name call matches update_person(uid, person_id, name) and is consistent with rename_person_retaining_aliases storing the clean name.
• backend/tests/routers/test_users.py — new cases cover strip, blank/whitespace-only, too-short, and >40, with update_person mocked and assert_not_called on rejection; the full file passes 100/100 in my run.
• backend/tests/unit/test_person_name_validation.py — hermetic (only imports models.other); passes 9/9.

One non-blocking thought: the inline checks in update_person_name duplicate the CreatePerson validator logic; if more person-name endpoints appear, extracting a shared helper would keep the bounds in one place. Not needed at this size.

This fixes a real correctness bug with clean, focused tests — nice first contribution. Leaving the final approve/merge to the maintainers.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[aiwithrajan]

Thanks @Git-on-my-level for the thorough sandbox verification and positive feedback! All tests are passing cleanly and the branch is conflict-free against main. Ready for final maintainer merge @undivisible.

[github-actions]

Hey @aiwithrajan 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our product principles and locked invariants

Before your next PR, please skim:

• PRODUCT.md — product north star
• Product invariants — locked rules (shared chat, memory tiers, agent control plane, integrations, brand)

If this was declined for direction or taste, maintainers should cite an invariant ID or open a proposed one — ask if that citation is missing.

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community!