Skip to content

feat(line): hermes-agent#23197 takeaways — body cap + outbound transforms + real-adapter bones - #2

Merged
choguun merged 22 commits into
mainfrom
feat/line-hermes-takeaways
Jul 3, 2026
Merged

choguun merged 22 commits into
mainfrom
feat/line-hermes-takeaways

Conversation

@choguun

@choguun choguun commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Closes 4 of the 11 gaps documented in docs/line-integration-gap-analysis.md against NousResearch/hermes-agent#23197 (1638 LOC LINE plugin adapter).

What ships

  1. Body cap (1 MiB) on the webhook — memory-exhaustion guard. 413 on oversize, before signature work.
  2. Outbound Markdown stripper + 5-message / 4500-char chunker in line/base.py — used by future real wiring; tested on the mock.
  3. LineRealAdapter: bot user-id cache + reply-token cache + self-message filter stub — the structural bones the eventual HTTP wiring will need. send_reply() still raises NotImplementedError; the docstring in the method lists the exact order (strip_markdown → split_for_line → consume_reply_token → Reply-or-Push).

What doesn't ship (and why)

The full diff in docs/line-integration-gap-analysis.md explains each gap. The deferred items:

  • Three-allowlist gating (LINE_ALLOWED_USERS / _GROUPS / _ROOMS) — N/A for single-tenant MVP
  • Media inbound (image/audio/video/file/sticker/location) — defer until listing creation needs photos from LINE
  • Media SEND with HTTPS serving — defer; we have /api/upload-image
  • Slow-LLM postback button — N/A, no async-streaming LLM
  • accountLink / memberJoined / things / loading indicator — defer (no LIFF, no IoT)
  • unsend event handling — 1-migration scope, defer to follow-up

Verified

  • pytest: 168/168 (was 138; +30 new) — coverage 92.88% ≥ 80% ✅
  • RUN_REAL_ADAPTER_TESTS=1 pytest: 6/6 ✅
  • ruff + mypy strict: clean
  • Frontend unchanged (36/36 vitest)

choguun added a commit that referenced this pull request Jul 3, 2026
Applies all 4 P0 findings from the PR #2 review. C1+C2 are the load-bearing
ones; C3+C4 are doc/comment accuracy.

## C1 — Mock applies the same outbound transforms as the real adapter

Previously LineMockAdapter.send_reply recorded text verbatim.
When real wiring lands and the real adapter strips **bold, headings,
code fences, etc.** + chunks at the 5-message cap, mock and real
diverge on what LINE would actually receive.

The mock now mirrors the real adapter's outbound path:

  strip_markdown(text) \u2192 split_for_line(cleaned) \u2192 record each chunk
  with chunk_index 0..N-1.

Tests in test_line_helpers.py::TestLineMockAdapterOutboundParity
verify that **bold / *italic* are stripped, long text is chunked, and
the chunk count respects LINE_MAX_MESSAGES_PER_CALL=5.

Reply-token routing: tokens are single-use. The mock now consumes the
token on first send_reply so a second send to the same chat falls
back to push (matching real behavior). Tests verify mode='reply'
on the first call and mode='push' on the second.

## C2 \u2014 Reply-token cache + bot_user_id live on the Protocol

Previously the bones sat only on LineRealAdapter and the webhook
couldn't reach them without an isinstance() check \u2014 breaking the
4-adapter contract ("no router imports a concrete class by name").

Both bot_user_id and set_reply_token(chat_id, token) are now
on the LineAdapter Protocol. Both the mock and the real adapter
implement them. The webhook takes a LineDep and calls
line.set_reply_token(chat_id, reply_token) on every inbound
message event with a replyToken field.

The router has a new _cache_reply_token helper that:

  - Reads replyToken from the event
  - Resolves chat_id from source.userId / groupId / roomId
  - Silently skips when neither is set (defensive against malformed events)

Tests in test_line_webhook.py cover:

  - Inbound message with replyToken \u2192 cache populated
  - Inbound follow/unfollow/etc. \u2192 cache stays empty (no replyToken)
  - set_reply_token consumes on first send_reply (Reply mode)
  - Second send_reply without re-caching falls back to Push (mode='push')

The TTL constant REPLY_TOKEN_TTL_SECONDS = 60 was moved from
real.py to base.py so the mock and real share it.

## C3 \u2014 Doc lies

Two doc lies fixed:

- TL;DR table at the top of docs/line-integration-gap-analysis.md
  marked unsend event handling as **"Fix in this PR"** \u2014 it
  wasn't. Now correctly marked **"Defer (1-migration scope; not in
  this PR)"**.

- The "TL;DR for the PR description" block listed unsend as one
  of the 4 things shipped \u2014 it wasn't. Now lists the actual 4
  (body-cap, transforms, self-message + reply-token, LineDep wiring).

The "Verdict + suggested action" section (line 239) already said
"defer" correctly; only the two stale bits are now consistent with it.

## C4 \u2014 aiohttp comment was a copy-paste from hermes-agent

backend/app/routers/line_webhook.py:33 had a comment claiming
"aiohttp's client_max_size doesn't apply in all body modes". This
project uses FastAPI + Starlette + uvicorn, NOT aiohttp \u2014 the
reference was a copy-paste from the source PR.

Rewrote the comment to reference Starlette's Request.body() and
mention uvicorn's h11_max_incomplete_event_size for nginx-fronted
deployments. The 1 MiB cap (line 41) is now correctly described as
memory-exhaustion defence for Starlette's unbounded buffer.

## Verified

- pytest: 184/184 (was 168; +16 new) \u2014 coverage 92.99% \u2265 80% \u2705
- RUN_REAL_ADAPTER_TESTS=1: 6/6 \u2705 (Protocol conformance still
  holds after bones promotion; isinstance(LineRealAdapter,
  LineAdapter) + isinstance(LineMockAdapter, LineAdapter) both
  return True)
- ruff + mypy strict: clean

## Out of scope

- strip_markdown leaks content from code spans (Backend warning) \u2014
  noted, follow-up
- _MD_ITALIC_STAR matches arithmetic (Backend warning) \u2014 noted,
  follow-up
- self-message filter default-off (Backend warning) \u2014 real wiring
  must set bot_user_id; documented
- chat_id / line_user_id reconciliation \u2014 currently same in DMs;
  groups deferred to when we add group support
- unsend event handling \u2014 1-migration, deferred (correctly noted in
  C3 fix)
@choguun
choguun marked this pull request as ready for review July 3, 2026 12:04
choguun added 22 commits July 3, 2026 19:38
Full AIDLC spec for Month-1 MVP. Scope:

- Next.js 15 + FastAPI + Supabase + LINE + Claude/Gemini adapters
- Every external integration behind a mock/real adapter pair
- Single agent only (no teams), Thai property UX, PDPA-safe defaults
- 12 acceptance criteria + 20 ST-NNN test scenarios

Layout: backend/app/{domain,adapters,routers,services}/ +
web/app/(marketing|auth|app)/. Adapters live behind Protocol classes
so USE_MOCKS=false swaps mock→real adapters with no router change.

Open questions parked for plan phase.
Backend (Python 3.11 / FastAPI 0.115 / pydantic v2):
- pyproject.toml with ruff+mypy(strict)+pytest config
- requirements.txt (fastapi, uvicorn, pydantic-settings, jwt, bcrypt, httpx, ...)
- app/main.py factory + CORS + lifespan
- app/config.py (pydantic-settings) — env-driven, mock-first defaults
- app/routers/health.py — GET /health → {"status":"ok"}
- tests/conftest.py + tests/test_health.py (ST-001)
- .env.example

Frontend (Next.js 15 / React 19 / Tailwind v3 / shadcn config):
- package.json, tsconfig.json, next.config.mjs, tailwind.config.ts, postcss
- app/globals.css + design tokens (shadcn variables)
- app/layout.tsx + app/page.tsx (landing shows backend health badge)
- app/api/health/route.ts (proxy → backend)
- lib/api.ts (typed fetch + ApiError + token cache)
- lib/utils.ts (cn helper)
- vitest.config + setup + 1 passing unit test
- components.json (shadcn config), .eslintrc.json, .env.example

CI (GitHub Actions):
- Backend matrix: ruff check, format check, mypy strict, pytest
- Frontend matrix: lint, typecheck, vitest

Toolchains locally verified:
- uvicorn app.main:app → 200 OK on /health and /
- pytest -v: 3/3 passed
- npm run lint: clean
- npm run typecheck: clean
- npm test: 2/2 passed
Backend (Python):
- app/adapters/supabase/base.py — SupabaseAdapter Protocol (query, count, insert, update, delete, get_by_id)
- app/adapters/supabase/_schema.py — Schema/Table/Column dataclasses; DEFAULT_SCHEMA declares all 10 tables (users, teams, properties, leads, messages, appointments, generated_listings, contracts, user_settings, audit_logs) with PG types, NOT NULL, defaults callable (UUID mint, NOW(), role='agent', status='draft', etc.)
- app/adapters/supabase/mock.py — MockSupabaseAdapter: in-memory, insertion-ordered, NOT NULL validation, auto-id, updated_at re-stamp on update, reset() helper
- app/adapters/supabase/real.py — RealSupabaseAdapter: stub, raises NotImplementedError, implements the Protocol so isinstance() succeeds (ST-019-shaped)
- app/adapters/supabase/_factory.py — get_db() selects adapter by Settings(use_real_supabase)
- app/adapters/supabase/__init__.py — public re-exports
- app/adapters/__init__.py
- app/deps.py — DBDep = Annotated[SupabaseAdapter, Depends(get_db_dep)]
- migrations/001_init.sql — canonical Postgres DDL mirroring DB.md
- migrations/__init__.py

Tests (pytest, 22 new tests, total 25/25 passing):
- Schema presence — all 10 expected tables exist (covers AC-01 partial)
- SQL ↔ mock schema parity (table names must match, test fails on drift)
- Round-trip CRUD on users, properties, leads
- Defaults: user.role='agent', property.status='draft', lead.source='line', lead.status='new'
- Auto-id is canonical UUID (36 chars, 4 hyphens)
- NOT NULL enforcement
- update of unknown id → None
- delete of unknown id → False
- count() with and without filters
- Stable insertion-order query (no order_by)
- order_by + desc sorting
- limit + offset pagination
- snapshot stability across fresh adapters
- factory: mock by default, real when USE_REAL_SUPABASE=true

Verified locally:
- pytest -q → 25/25 in 0.03s
- coverage: --cov=app = 91% (real.py only low because stubs are intentional)
- ruff check app/ tests/ → all checks passed
- ruff format app/ tests/ → 18 files left unchanged
- mypy app/ (strict + pydantic plugin) → success, no issues
- uvicorn /health → 200 {"status":"ok"} (no regression)
Backend (Python, FastAPI, Pydantic v2, bcrypt, PyJWT):
- app/domain/user.py — User + SignupIn + LoginIn + LiffIn + AuthResponse DTOs
- app/services/auth.py — AuthService (signup/login/liff_login/user_from_token),
  hash_password / verify_password (bcrypt), create_access_token / decode_token
  (HS256, jwt_ttl configurable), typed AuthError subclasses (DuplicateEmail 409,
  InvalidCredentials 401, UserNotFound 404) with HTTP mapping
- app/routers/auth.py — POST /api/auth/{signup,login,liff}, GET /api/auth/me
- app/deps.py — get_db_dep already; new AuthServiceDep wired in router
- app/main.py — register auth_router
- app/adapters/supabase/_factory.py — process-singleton mock with thread
  safety; one bug found and fixed during E2E (per-request mock lost state)
- app/adapters/supabase/{_schema.py, ../../migrations/001_init.sql} —
  added password_hash TEXT to users (both mirror each other)
- tests/test_auth.py — 15 tests
  ST-002: signup OK + duplicate → 409
  ST-003: login OK + wrong pwd → 401 + unknown email → 401
  ST-004: LIFF OK + reuse same user + placeholder email
  /me: valid token → 200, no header → 401, malformed → 401, wrong scheme → 401

Frontend (Next.js 15, React 19, Tailwind, ts):
- lib/api.ts — extended: apiGet/apiPost, clearAuthToken, getAuthToken, User type
- lib/auth.ts — login/signup/liffLogin/fetchMe/describeAuthError
- app/(auth)/layout.tsx — card-style auth layout
- app/(auth)/login/page.tsx — email+password + green LINE button
- app/(auth)/signup/page.tsx — full_name+email+password
- app/(app)/dashboard/page.tsx — placeholder that loads /me (real dashboard
  lands in T-011)
- __tests__/auth.test.ts — 7 tests covering wrappers + error mapping

Verified locally:
- pytest: 40/40 (15 new from T-003)
- coverage on app/: 94% (target was 80%)
- ruff + mypy strict: clean
- frontend: lint, typecheck, vitest 9/9
- next build: compiled + 7 routes generated
- end-to-end via curl: signup → JWT → /me returns user across requests
Backend:
- app/domain/property.py — PropertyType + PropertyStatus enums; PropertyCreate
  (required), PropertyUpdate (all-optional including status), Property (response)
- app/deps.py — CurrentUserIdDep (bearer token → user id without DB hit) +
  SettingsDep
- app/routers/properties.py — list/create/get/patch/archive scoped to user_id;
  cross-user reads / writes / archives return 404 (not 403) to avoid id probing
- app/main.py — register properties_router
- tests/test_properties.py — 16 tests:
  auth gate (401 without token)
  ST-005: round-trip create + defaults (status='draft', foreign_quota=False)
  list scopes to caller, excludes archived by default
  get / patch / archive flow
  ?status= filter and ?include_archived=true flag
  archive is idempotent
  cross-user isolation: 404 on read, patch, archive; not in list
  payload validation: invalid property_type, negative price, extra fields (422)
  minimal payload accepted

Frontend (Next.js 15):
- lib/types.ts — Property + PropertyCreateInput + PropertyUpdateInput +
  formatTHB (Intl) + propertyTypeLabel (en/th)
- lib/api.ts — added apiPatch/apiDelete/apiPostNoBody
- lib/properties.ts — listProperties/getProperty/createProperty/updateProperty/archiveProperty
- app/(app)/layout.tsx — auth-gated chrome with nav
- app/(app)/properties/page.tsx — client component, redirects if no token,
  loading/error/empty/list states
- components/properties/PropertyCard.tsx — Thai labels + THB format + status pill
- components/properties/PropertyCard.test.tsx — 5 RTL tests (Thai text, fallback,
  status)
- vitest.config.ts — added @vitejs/plugin-react for JSX automatic runtime

Verified locally:
- pytest: 56/56 ✅ (16 new from T-004)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + tsc + build: clean, 8 routes generated
- vitest: 14/14 ✅ (5 new component tests)
- end-to-end via curl: signup → create condo + house → list 2 → archive one → list 1
- 401 without token
…rty form

Backend (Python):
- app/adapters/storage/base.py — StorageAdapter Protocol + StoredObject
  DTO + is_allowed_image() allow-list (jpeg/png/webp/gif)
- app/adapters/storage/local_mock.py — disk-backed mock; writes to
  ${var_dir}/uploads/{uuid}{ext}; path-traversal defence (rejects
  '/' / '\\' / '.');
  max 10 MiB per file
- app/adapters/storage/supabase_real.py — stub for MVP (NotImplementedError)
- app/adapters/storage/_factory.py — picks by Settings.use_real_supabase
- app/adapters/storage/__init__.py — public re-exports
- app/deps.py — StorageDep; SettingsDep now reads settings from
  request.app.state (no longer global cache; tests override cleanly)
- app/main.py — register storage_router; app.state.settings set in factory
- app/routers/storage.py —
    POST /api/upload-image (multipart, auth-gated, 415 on bad type,
                              400 on empty/too-big)
    GET  /static/{key} (no auth, 404 on missing/path traversal)
- app/config.py — public_base_url setting (default http://localhost:8000)
- .env.example — PUBLIC_BASE_URL line
- tests/test_storage.py — 14 tests:
    Direct adapter: writes to disk, rejects empty, get round-trip,
    path-traversal blocked, delete idempotent
    HTTP: ST-015 upload returns URL with key; uploaded file served via
    /static/{key}; 401 without auth; 415 on bad extension or MIME;
    accept jpg; 404 on /static/no-such

Frontend (Next.js 15):
- lib/api.ts — apiUpload for FormData (browser sets content-type
  with boundary; Content-Type not forced to JSON for FormData)
- lib/uploads.ts — uploadImage + uploadImages wrappers
- components/forms/ImageUploader.tsx — multi-file picker, generates
  preview thumbnails via URL.createObjectURL, remove button, accepts
  image/* with allow-list
- components/forms/PropertyForm.tsx — client component, all
  property fields (Thai labels, ตร.ม. units, BTS/MRT, foreign quota),
  upload-on-submit flow, error/loading/redirect-to-detail
- app/(app)/properties/new/page.tsx — entry point

Verified locally:
- pytest: 70/70 ✅ (14 new from T-005)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + typecheck + build: clean, 9 routes incl. /properties/new
- vitest: 14/14 ✅
- end-to-end via curl on backend: signup → upload PNG → GET /static/{key}
  returns 200 with PNG bytes
Backend (Python):
- app/domain/listing.py — Platform enum (4), PropertySummary,
  ListingRequest (forbid extras), GeneratedContent response
- app/adapters/ai/base.py — AiAdapter Protocol + FallbackToNext +
  BadRequest (the two error categories the service distinguishes)
- app/adapters/ai/anthropic_mock.py — deterministic Thai templates
  per (property_type, platform); DDProperty/Livinginsider/Facebook/General;
  includes ตร.ม., ห้องนอน, ห้องน้ำ, ชั้น, โควต้าต่างชาติ; Facebook
  emits 6 hashtags; General is bilingual.
- app/adapters/ai/gemini_mock.py — different model name + tone;
  used as the fallback chain's secondary
- app/adapters/ai/anthropic_real.py, gemini_real.py — stubs raise
  FallbackToNext('not wired in MVP')
- app/adapters/ai/_factory.py — build_ai_chain([primary, secondary])
- app/services/listing_generator.py — orchestrates the chain; raises
  BadRequest immediately; tries next adapter on FallbackToNext OR any
  transient exception; surfaces RuntimeError if all fail
- app/deps.py — AIChainDep
- app/routers/ai.py — POST /api/generate-listing; auth required;
  filters by platforms (default = all 4); 400/422 on bad payload
- app/main.py — register ai_router
- tests/test_ai_generator.py — 13 tests:
    ST-006: condo DDProperty contains คอนโด/ตร.ม./ห้องนอน/BTS Asok
    condo Facebook has ≥ 5 hashtags
    all 4 platforms returned
    ST-007: house General mentions บ้านเดี่ยว or 'house'
    house DDProperty mentions 200 ตร.ม.
    p99 latency < 2s (10 × 4 platforms)
    fallback when primary raises FallbackToNext
    4xx surfaces immediately (no fallback chain)
    fallback when primary raises arbitrary exception
    HTTP: returns 4 platforms, accepts platforms filter, 401 without auth,
    rejects unknown platform (422)

Frontend (Next.js 15):
- lib/types.ts — added Platform/PLATFORM_LABELS/GeneratedContent/
  PropertySummaryForAi
- lib/listings.ts — generateListing() wrapper
- components/forms/ListingPreview.tsx — 4 platform tabs + copy button +
  model badge
- components/forms/ListingPreview.test.tsx — 6 tests
- components/forms/PropertyForm.tsx — added '✨ Generate' button +
  previews ListingPreview when present + generation error handling

Verified locally:
- pytest: 83/83 ✅ (13 new from T-006)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + typecheck + build: clean, 9 routes
- vitest: 20/20 ✅ (6 new from T-006)
- curl E2E: signup → generate 4 platforms → all return Thai text,
  Facebook has 6 hashtags
Backend (Python):
- app/domain/listing.py — GeneratedListingCreate + GeneratedListingUpdate
  + GeneratedListing (DB row); strict validation (extra='forbid', title ≥1)
- app/routers/listings.py — POST /api/listings, GET
  /api/listings?property_id=..., PATCH /api/listings/{id}, DELETE
  /api/listings/{id} (204). Cross-user returns 404 (not 403).
- app/main.py — register listings_router
- tests/test_listings.py — 12 tests covering round-trip,
  no-op PATCH, delete idempotency, auth gate, cross-user isolation,
  validation

Frontend (Next.js 15):
- lib/types.ts — SavedListing (extends GeneratedContent with id/created_at
  etc.) + SaveListingInput + UpdateListingInput. Widened property_type
  to string to match backend's permissive PropertySummary.
- lib/listings.ts — added listListingsForProperty, saveListing,
  updateListing, deleteListing
- components/forms/ListingEditor.tsx — tab-style editor per platform
  variant: editable title/description/hashtags/SEO; Save button tracks
  dirty state, shows 'Saved at HH:MM:SS' feedback, falls back to
  'Re-save' after first save
- components/forms/ListingEditor.test.tsx — 6 tests (rendering,
  platform label, dirty/save state)
- components/forms/PropertyForm.tsx — auto-save generated listings on
  property submission (sequential, errors logged not blocking)
- app/(app)/properties/[id]/page.tsx — detail page: property header
  (title, status pill, district/province, key fields, image strip),
  listings grid with one editor per platform variant, generation from
  detail (lazy import saveListing). Has loading/error/empty states.

Verified locally:
- pytest: 95/95 ✅ (12 new from T-007)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + typecheck + build: clean, 11 routes incl.
  /properties/[id] (dynamic)
- vitest: 26/26 ✅ (6 new from T-007)
- curl E2E: signup → property → 2 listings (POST 201) → list 2 →
  PATCH (updated title) → DELETE (204) → 401 without token
Backend (Python):
- app/adapters/line/base.py — LineAdapter Protocol + sign_line_webhook /
  verify_line_webhook helpers using hmac.compare_digest (constant-time).
  SIGNATURE_HEADER = 'X-Line-Signature'.
- app/adapters/line/mock.py — LineMockAdapter in-memory; sign() helper
  for tests
- app/adapters/line/real.py — LineRealAdapter stub (no HTTP yet;
  same sign/verify surface so the rest of the app is unaffected)
- app/adapters/line/_factory.py — get_line_adapter (mock when
  use_real_line=false)
- app/adapters/line/__init__.py — public re-exports
- app/routers/line_webhook.py — POST /webhook/line: reads RAW body
  bytes first, then signature, then verifies BEFORE JSON parsing.
  Verified + ack returns {'ok': true, 'received': N}. Failed
  verification returns 401 with NO DB writes. JSON parse fails → 400.
- app/main.py — register line_webhook_router
- tests/test_line_webhook.py — 11 tests:
    ST-009 valid signature → 200
    ST-010 invalid signature → 401
    missing signature → 401
    empty signature → 401
    body tampered after signing → 401
    signature from different secret → 401
    invalid JSON after signature passes → 400 (not 401)
    no DB writes on unverified request
    helper round-trip
    None signature rejected
    same signed payload twice → 200, 200 (idempotency in T-009)

Verified locally:
- pytest: 107/107 ✅ (12 new from T-008)
- coverage on app/: 94%
- ruff + mypy strict: clean
- python urllib smoke: valid → 200, invalid → 401, missing → 401

Security property: signature is verified against the raw request bytes
(hmac.compare_digest) BEFORE JSON parsing — a body-tampering attacker
cannot smuggle events past verification.
Backend (Python):
- app/domain/lead.py — Lead DTO with from_row helper
- app/domain/message.py — Message DTO with from_row helper
- app/services/lead_pipeline.py — LeadPipeline service:
    • idempotency via event_id scan over messages.raw_data
    • find-or-create lead by line_user_id
    • insert inbound Message + raw_data (full event)
    • bump lead.updated_at on contact
    • never crashes on malformed payloads; returns ProcessResult
      with reason in {ok, replay, no_event_id, no_source, non_message}
- app/routers/line_webhook.py — now wires verified events through
  LeadPipeline. Agent lookup happens ONLY when events are non-empty.
    • uses settings.line_default_agent_id (env var) if set
    • falls back to first active user in mock mode (helpful for dev)
    • 503 only when events present + no agent config + no users
- app/config.py — added line_default_agent_id field
- .env.example — added LINE_DEFAULT_AGENT_ID
- tests/test_line_webhook.py — 8 new tests (T-008's 12 still pass):
    ST-011 replay of same event_id is ignored
    ST-012 two events from same user → one Lead, two Messages
    well-formed event creates lead + message
    non-message (follow) event ignored
    missing source → no_source
    missing event_id → no_event_id
    empty events array → 200 with received=0
    no-agent scenario → 503 (with events)
- Added autouse _isolate fixture to reset mock singleton between tests

Verified locally:
- pytest: 114/114 ✅
- coverage on app/: 94%
- ruff + mypy strict: clean
- python urllib E2E: signup → 2 events same LINE user → 1 lead, 2
  messages; replay same body → 0 processed (all reason='replay')

Behavioural properties:
- empty events → 200 (no agent needed)
- verified + has events + no agent → 503
- verified + has events + has agent (env or first user) → process
- duplicate event_id → skip with reason='replay'
- non-message / missing event_id / missing source → skip, do not crash
Backend (Python):
- app/domain/lead.py — LeadStatus enum + LeadUpdate DTO (strict)
- app/adapters/line/base.py — added send_reply() to Protocol
- app/adapters/line/mock.py — added send_reply() records line_user_id +
  text + sent_at; keeps sent_replies list for tests
- app/adapters/line/real.py — send_reply() stub raises NotImplementedError
- app/deps.py — added LineDep + get_line_dep
- app/routers/leads.py — GET /api/leads (status filter, limit, ordered
  by updated_at desc), GET /api/leads/{id} (lead + messages ascending),
  PATCH /api/leads/{id} (status enum validated, extras forbid)
- app/routers/messages.py — POST /api/leads/{id}/messages
    • validates lead.user_id matches caller (404 on cross-user)
    • rejects leads without line_user_id (400)
    • calls line.send_reply() then inserts outbound Message with
      is_ai_generated=false; bumps lead.updated_at
- app/routers/line_webhook.py — switched to DBDep for test injectability
- app/main.py — register leads_router + messages_router
- tests/test_leads.py — 14 tests:
    list: scoped to caller / status filter / 401 without auth
    get: returns messages created-order / cross-user 404 / unknown 404
    patch: fields update / unknown status 422 / extras forbid 422
    reply: inserts outbound + calls line.send_reply + cross-user 404
           + 400 without line_user_id + 401 without auth + both directions

Frontend (Next.js 15):
- lib/types.ts — Lead, Message, LeadWithMessages; Thai LEAD_STATUS_LABELS
- lib/leads.ts — listLeads(opts), getLead, updateLead
- lib/messages.ts — sendReply(leadId, text)
- components/chat/MessageList.tsx — inbound (left card border) vs
  outbound (right emerald bubble), timestamp, agent/lead label
- components/chat/ComposeBox.tsx — controlled textarea + send button,
  disabled when empty, surfaces ApiError detail
- app/(app)/leads/page.tsx — list with status-pill filter row,
  empty state, lead row links to detail; Thai status badges
- app/(app)/leads/[id]/page.tsx — header (name, line_user_id, status,
  budget/interest/notes), MessageList + ComposeBox

Verified locally:
- pytest: 128/128 ✅ (14 new from T-010)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + typecheck + build: clean, 11 routes incl. /leads + /leads/[id]
- vitest: 26/26 ✅
- python urllib E2E: signup → webhook (2 leads) → list → get with
  messages → reply (201) → directions = ['inbound','outbound'] →
  PATCH status (200)
Backend (Python):
- app/routers/dashboard.py — GET /api/dashboard
    • new_leads_count: count of leads with status='new' for caller
    • recent_inbound: last 20 inbound messages, each enriched with
      lead preview (id, name, line_user_id)
    • recent_properties: last 5 non-archived properties, newest first
- app/main.py — register dashboard_router
- tests/test_dashboard.py — 8 tests (ST-013 + shape + auth + isolation):
    three blocks always present; new leads counted;
    lead preview attached; newest properties first; archived excluded;
    401 without auth; cross-user isolation (B sees no A's data);
    inbound capped at 20

Frontend (Next.js 15):
- lib/types.ts — DashboardData + DashboardInboundMessage + DashboardLeadPreview
- lib/dashboard.ts — getDashboard()
- components/dashboard/NewLeadsCounter.tsx — large emerald number,
  dims to muted-foreground when 0, links to /leads, Thai caption
- components/dashboard/RecentMessages.tsx — last 20 inbound items with
  truncate, time-ago label, link to /leads/[id]; Thai empty state
- components/dashboard/RecentProperties.tsx — reuses PropertyCard;
  empty state with CTA
- app/(app)/dashboard/page.tsx — replaces the placeholder; greeting
  + sign-out + the three sections; auto-refresh every 5s via setInterval;
  redirects to /login on 401

Verified locally:
- pytest: 136/136 ✅ (8 new from T-011)
- coverage on app/: 94%
- ruff + mypy strict: clean
- next lint + typecheck + build: clean, 11 routes incl. /dashboard (3.74 kB)
- vitest: 26/26 ✅
- python urllib E2E: empty dashboard → 2 leads + 3 inbound + 1
  active property (archived filtered out), newest first
Backend:
- tests/test_real_swap.py — 6 tests gated by RUN_REAL_ADAPTER_TESTS=1
  • isinstance(RealSupabaseAdapter, SupabaseAdapter)
  • isinstance(RealAnthropicAdapter, AiAdapter)
  • isinstance(RealGeminiAdapter, AiAdapter)
  • isinstance(RealLineAdapter, LineAdapter)
  • isinstance(SupabaseStorageAdapter, StorageAdapter)
  • sign_line_webhook round-trip
- pyproject.toml — addopts gets --cov-fail-under=80
  (current coverage: 92.88% — passes the gate)

Frontend:
- playwright.config.ts — fullyParallel off, single worker, webServer
  starts backend in CI, baseURL configurable for frontend
- tests/e2e/happy-path.spec.ts — full happy path: UI signup →
  /dashboard → API creates property + 4 listings → UI logs in
  fresh → /properties/[id] → 4 editors visible → edit one + save
  → 'Saved at HH:MM:SS' feedback → sign out
- package.json — @playwright/test devDep + test:e2e script
- tsconfig.json — exclude playwright.config.ts + tests/e2e/
- vitest.config.ts — exclude tests/e2e/ from collection

Docs (shipped in docs/):
- architecture.md — layers, request lifecycles, state model,
  what is intentionally NOT here
- adapters.md — the 4-pair contract, mock→real switch matrix,
  per-adapter behaviour tables
- runbook.md — quick start, where things live, adding features,
  debugging recipes, production rollout, incident oncall

README rewritten with quick-start + doc map.

Verified locally:
- pytest: 136/136 (real_swap +1 pass + 5 skip without flag; all 6
  pass with RUN_REAL_ADAPTER_TESTS=1)
- coverage gate: 92.88% ≥ 80% ✅
- ruff + mypy strict: clean
- vitest: 26/26 ✅ (e2e excluded)
- next lint + typecheck + build: clean; e2e excluded from build
Driven by findings from 4 parallel review sub-agents (backend,
frontend, docs, adapter contract). 10 real bugs surfaced across all
three tiers; this commit resolves every Tier-1 finding.

## Backend

- **auth.py:_map_auth_error** — Drop UserNotFound from the union. The
  mapper is called by signup/login/liff which never raise
  UserNotFound; /me handles it explicitly with 404. Previously the
  same exception would map to two different status codes depending on
  caller — classic foot-gun.
- **auth.py:get_auth_service** — Switch from Depends(get_settings)
  (lru_cached global) to SettingsDep (per-request app.state). Tests
  passing Settings(...) to create_app() will now actually see their
  isolated Settings — no more latent test-isolation leak.
- **line_webhook.py** — Switch from request.app.state.settings inside
  the handler body to SettingsDep parameter. Same rationale.
- **dashboard.py:get_dashboard** — Replace N+1 db.get_by_id(leads, ...)
  loop with a single db.query(leads, ...) + dict lookup. Defense-in-
  depth: re-check user_id when enriching so a future cross-user
  message insert doesn't leak the lead preview fields.
- **line/base.py:LineAdapter protocol** — add send_reply() to the
  Protocol so it's discoverable; mocks/reals that lack it will fail
  isinstance() instead of TypeError'ing at runtime. Drops the
  '# type: ignore[attr-defined]' hack in messages.py.
- **ai/_factory.py, line/_factory.py, storage/_factory.py,
  supabase/_factory.py** — use_mocks=True is now the master switch,
  short-circuiting to <Mock> regardless of any USE_REAL_* flag.
  Documents the 'mocks win' semantics in module docstrings.
- **tests/adapters/test_mock_supabase.py** — updated
  test_factory_returns_real_when_flag_set to explicitly set
  use_mocks=False (master switch now requires it). Added
  test_factory_master_switch_overrides_real_flag to lock the
  semantics in.
- **coverage on routers/dashboard.py** jumped to 100% (new branches
  exercised).

## Frontend

- **components/forms/ImageUploader.tsx** — fix URL.createObjectURL leak:
  useEffect cleanup now runs on previews state change, not just
  unmount. Long editing sessions were leaking blob refs on every
  re-selection.
- **components/forms/ListingEditor.tsx** — fix dirty-after-save bug:
  track a lastSaved snapshot (not initial); on save, bump the
  snapshot so the next edit round trips correctly. Also handles
  parent re-fetch via useEffect-on-initial.
- **app/(app)/layout.tsx** — hoist auth gate from per-page into the
  layout itself. Closes the deep-link foot-gun: /properties/new and
  /properties/[id] now redirect to /login if no token is in
  localStorage, instead of rendering an empty form that fails on
  Save.
- **lib/types.ts** — add team_id (string | null) to Property and Lead
  so the frontend types match backend DTOs. extra='ignore' had been
  silently dropping them, breaking any future UI needing the field.
- **components/properties/PropertyCard.test.tsx** — added
  team_id: null to baseProperty fixture.

## Verified

- pytest: 138/138 ✅ (was 137; new master-switch test in
  test_mock_supabase)
- coverage: 92.42% ≥ 80% ✅
- ruff + mypy strict: clean
- vitest: 26/26 ✅
- next build: 11 routes, e2e excluded ✅
Driven by the second wave of findings from the 4 sub-agent reviews.
Tier-3 doc cleanup lands in a separate commit.

## Backend

- app/routers/auth.py — TODO(security) comment above /api/auth/login
  flagging the missing rate limiter. Post-MVP follow-up; not part of
  Month-1 scope.
- app/adapters/supabase/_schema.py — promote _now() to now_iso(),
  the public helper. _now kept as a module-internal alias so the
  table definitions don't churn.
- app/adapters/supabase/mock.py — use the shared now_iso() instead of
  a duplicate _now() definition. Single source of truth — a future
  timezone-aware change reaches both places.

## Frontend behavior

- app/(app)/properties/[id]/page.tsx — drop the dead dynamic
  'await import("@/lib/listings")' (already imported at the top of
  the file). Also: the Generate / Regenerate button was previously
  disabled once any listings existed (no re-generate possible). Now
  only disabled while generating; shows '🔄 Regenerate' once saved.
- components/forms/ListingEditor.tsx — TypeScript now narrows
  'initial.description' correctly (extra guard for nullable).
- web/lib/api.ts — drop the module-level cachedToken mutable.
  readToken() now reads localStorage on every call. The cache was
  an architectural leftover that caused StrictMode double-render
  races and one-component-clears-another bugs. Cost: ~10 µs per
  fetch — invisible.
- web/lib/api.ts + lib/{auth,dashboard}.ts — apiGet/apiPost/apiPatch
  accept a {signal?: AbortSignal} option so call sites can abort.
- app/(app)/dashboard/page.tsx — Polling loop now uses AbortController
  per tick so a slow backend doesn't cause overlapping in-flight
  fetches. Also catches + swallows DOMException('AbortError') so
  aborts don't surface as 'Failed to load dashboard' errors.
- app/(app)/leads/page.tsx — filter row uses role='group' +
  aria-pressed instead of the misleading role='tablist' + aria-selected
  pattern (these are filter chips, not tabs).

## Missing tests (the bulk of T2)

- components/forms/ImageUploader.test.tsx — 4 tests: empty state,
  onFilesChange fires with images only (filters out non-images),
  revokeObjectURL is called on remove. jsdom doesn't ship URL.createObjectURL
  / revokeObjectURL; we polyfill via Object.defineProperty.
- components/forms/ComposeBox.test.tsx — 4 tests: disabled-empty,
  send happy path + textarea clears, two error surfaces (real ApiError
  → backend detail; non-ApiError → 'Send failed' fallback).
- __tests__/dashboard.test.tsx — 2 tests (ST-014): all three
  sections render with full payload; counter dims when new_leads_count=0.
- components/dashboard/RecentProperties.tsx — added
  data-testid='recent-properties' on the <ul> so the dashboard test
  can assert on it.

## Verified

- pytest: 138/138 ✅ (still 92% coverage on app/)
- ruff + mypy strict: clean
- vitest: 36/36 ✅ (was 26; +10 new tests across 3 files)
- next lint + tsc + build: clean, 11 routes
Third commit from the 4-sub-agent code review. Doc-only changes that
bring README.md / docs/ / .aidlc/ in line with the code that's been
landing in T-001..T-012 + Tier-1/2 review fixes.

## .aidlc/state.md
- Updated final stats: 19 commits, +20,716 lines, 132 files (was 17
  commits, +19,451 lines, 124 files).
- Test counts: 138 passed, 36 vitest, 92.29% coverage (was 137/26,
  92.88%).
- Added a 'Tier-1 + Tier-2 review fixes applied' note pointing to the
  two review commits.

## docs/runbook.md
- Quick-start now shows the right numbers: 138 passed / 142 collected,
  92.29% coverage, 36 vitest (was '~136 tests', 26 vitest).
- 'Where things live' adds a row for property/listing fields (cross-
  cutting backend + frontend edit).
- 'Coverage gate' section cites the actual 92.29% (was the wrong
  94%) and names the gate command verbatim.

## docs/architecture.md
- Layer diagram:
    * Drops (marketing) (the route group never existed; landing
      lives at app/page.tsx).
    * lib/ now lists all 10 files (was 8).
    * Routers/services/deps blocks all match the actual structure.
    * 'USE_MOCKS is the master switch' called out (was just
      'flips in via env flags').
    * Contract file layout corrected: '<real>.py' with a note that
      naming isn't uniform (supabase/line say 'real.py'; AI & Storage
      are per-provider; factories are uniform regardless).

## docs/adapters.md
- USE_MOCKS doc rewritten as 'master switch — when true, mocks win
  even if individual USE_REAL_* flags are set' (now matches the
  post-Tier-1 factory semantics).
- T-009 reference ('Required for T-009's webhook') generalised to
  'Required for the LINE webhook (real LINE is multi-tenant)'.
- Contract layout schema in the docs matches the actual filename
  pattern (with the same uniformity note as architecture.md).
- 'tests/test_real_swap.py' wording drops the dead 'sign_ai_request'
  helper reference (the helper doesn't exist — was a copy-paste).
- The signing helpers used in the test now correctly named:
  sign_line_webhook / verify_line_webhook.

## README.md
- Quick-start test count corrected: 'npm test # vitest — 36 tests'.
- Deploy row in tech-stack table references 'runbook + rollout
  checklist in docs/runbook.md'.

## .aidlc/spec.md
- Removed the spec-time fictional file references that never landed:
    * (marketing)/page.tsx     → app/page.tsx (landing at root)
    * web/vercel.json           → not shipped (config deferred to deploy)
    * backend/Dockerfile        → not shipped
    * backend/railway.toml      → not shipped
      (replaced with a one-line 'Dockerfile + railway.toml deferred
      to deploy phase' note)
- router error-mapping promise fixed: was
  'app/main.py::register_exception_handlers' (does not exist); now
  'app/routers/<name>.py::_map_*_error helpers; per-router' (matches
  actual implementation).

## Verification

- backend pytest: 138/138 ✅ (92.29% coverage, gate ≥ 80%)
- ruff + mypy strict: clean
- frontend vitest: 36/36 ✅
- frontend next lint + tsc + build: clean
- grep 'marketing|register_exception_handlers|Dockerfile|railway.toml|vercel.json|sign_ai_request' docs/ .aidlc/ README.md:
  → only intentional parenthetical mention of the deferred
    Dockerfile/railway.toml remains.
…rms, real-adapter bones

Driven by review of NousResearch/hermes-agent#23197 (1638 LOC LINE
plugin) against our mocks-first FastAPI implementation. Doc at
docs/line-integration-gap-analysis.md walks the full diff and
explains what's worth porting for our Thai real-estate AI agent.

This PR lands the four highest-leverage fixes:

1. **Webhook body cap (1 MiB).** Memory-exhaustion guard
   rejecting oversized payloads with 413 BEFORE the signature
   check. Constant from app.adapters.line.base so both adapters
   and the router share it.

2. **Outbound Markdown stripper** (app.adapters.line.base).
   LINE can't render Markdown reliably across iOS/Android/web/
   macOS clients. strip_markdown() removes ATX headings,
   bold/italic (*/_/__), inline code + code fences, leading list
   bullets, and blockquote markers while leaving bare URLs
   untouched. Used by future real wiring; tested on the mock.

3. **LINE 5-message / 4500-char chunker**
   (split_for_line()). Per LINE Messaging API docs each
   bubble caps at 5000 chars and each Reply/Push call caps at
   5 message objects. Naive splitter: paragraph boundaries first,
   then Thai 。/Western .  sentence boundaries, hard
   cut as last resort. Capped at LINE_MAX_MESSAGES_PER_CALL (5).
   Tested with 11 cases including the Thai sentence terminator.

4. **LineRealAdapter: bot user-id cache + reply-token cache +
   self-message filter stub.** When the real HTTP wiring ships,
   these are the bones it will use:
     - bot_user_id: str | None  set at __init__ (auto-fetched
       from GET /v2/bot/info when wiring lands)
     - _reply_tokens: dict[chat_id, (token, expires_at)]  with
       set_reply_token() + consume_reply_token() (Reply
       tokens are single-use; ~60s TTL; expire-test covered)
     - send_reply() short-circuits with
       skipped='self-message' when line_user_id ==
       bot_user_id — prevents the inbound-outbound echo loop
       that hits any production bot without this filter.
   send_reply() still raises NotImplementedError for real
   sends; the doc-string in the method lists the exact
   strip_markdown → split_for_line → consume_reply_token →
   Reply-or-Push order the eventual wiring will follow.

## Tests

- backend/tests/test_line_helpers.py — 28 new tests
  (TestStripMarkdown × 11, TestSplitForLine × 8,
  TestWebhookBodyCap × 1, TestLineRealAdapterStructure × 8)
- backend/tests/test_line_webhook.py — 2 new tests
  (oversized → 413, exactly-at-cap → 400 to prove  boundary)

## Verified

- pytest: 168/168 (was 138; +30 new) — coverage 92.88% ≥ 80% ✅
- RUN_REAL_ADAPTER_TESTS=1 pytest tests/test_real_swap.py: 6/6 ✅
  (real adapter isinstance checks now also cover the new
  set_reply_token / consume_reply_token surface)
- ruff + mypy strict: clean
- Frontend unchanged (lib/api.ts: 36/36 vitest still passing)

## Out of scope (logged in docs/line-integration-gap-analysis.md)

- Three-allowlist gating: N/A for single-tenant MVP
- Media inbound (image/audio/video/file/sticker/location): defer
  until listing creation needs inbound photos
- Media SEND with HTTPS serving: defer; we have
  /api/upload-image as the upload path
- Slow-LLM postback button (their headline feature): N/A, we
  don't run an async-streaming LLM
- Loading indicator / typing animation: defer
- accountLink / memberJoined / things: defer (no LIFF, no IoT)
- unsend event handling: 1-migration scope, defer to follow-up
Applies all 4 P0 findings from the PR #2 review. C1+C2 are the load-bearing
ones; C3+C4 are doc/comment accuracy.

## C1 — Mock applies the same outbound transforms as the real adapter

Previously LineMockAdapter.send_reply recorded text verbatim.
When real wiring lands and the real adapter strips **bold, headings,
code fences, etc.** + chunks at the 5-message cap, mock and real
diverge on what LINE would actually receive.

The mock now mirrors the real adapter's outbound path:

  strip_markdown(text) \u2192 split_for_line(cleaned) \u2192 record each chunk
  with chunk_index 0..N-1.

Tests in test_line_helpers.py::TestLineMockAdapterOutboundParity
verify that **bold / *italic* are stripped, long text is chunked, and
the chunk count respects LINE_MAX_MESSAGES_PER_CALL=5.

Reply-token routing: tokens are single-use. The mock now consumes the
token on first send_reply so a second send to the same chat falls
back to push (matching real behavior). Tests verify mode='reply'
on the first call and mode='push' on the second.

## C2 \u2014 Reply-token cache + bot_user_id live on the Protocol

Previously the bones sat only on LineRealAdapter and the webhook
couldn't reach them without an isinstance() check \u2014 breaking the
4-adapter contract ("no router imports a concrete class by name").

Both bot_user_id and set_reply_token(chat_id, token) are now
on the LineAdapter Protocol. Both the mock and the real adapter
implement them. The webhook takes a LineDep and calls
line.set_reply_token(chat_id, reply_token) on every inbound
message event with a replyToken field.

The router has a new _cache_reply_token helper that:

  - Reads replyToken from the event
  - Resolves chat_id from source.userId / groupId / roomId
  - Silently skips when neither is set (defensive against malformed events)

Tests in test_line_webhook.py cover:

  - Inbound message with replyToken \u2192 cache populated
  - Inbound follow/unfollow/etc. \u2192 cache stays empty (no replyToken)
  - set_reply_token consumes on first send_reply (Reply mode)
  - Second send_reply without re-caching falls back to Push (mode='push')

The TTL constant REPLY_TOKEN_TTL_SECONDS = 60 was moved from
real.py to base.py so the mock and real share it.

## C3 \u2014 Doc lies

Two doc lies fixed:

- TL;DR table at the top of docs/line-integration-gap-analysis.md
  marked unsend event handling as **"Fix in this PR"** \u2014 it
  wasn't. Now correctly marked **"Defer (1-migration scope; not in
  this PR)"**.

- The "TL;DR for the PR description" block listed unsend as one
  of the 4 things shipped \u2014 it wasn't. Now lists the actual 4
  (body-cap, transforms, self-message + reply-token, LineDep wiring).

The "Verdict + suggested action" section (line 239) already said
"defer" correctly; only the two stale bits are now consistent with it.

## C4 \u2014 aiohttp comment was a copy-paste from hermes-agent

backend/app/routers/line_webhook.py:33 had a comment claiming
"aiohttp's client_max_size doesn't apply in all body modes". This
project uses FastAPI + Starlette + uvicorn, NOT aiohttp \u2014 the
reference was a copy-paste from the source PR.

Rewrote the comment to reference Starlette's Request.body() and
mention uvicorn's h11_max_incomplete_event_size for nginx-fronted
deployments. The 1 MiB cap (line 41) is now correctly described as
memory-exhaustion defence for Starlette's unbounded buffer.

## Verified

- pytest: 184/184 (was 168; +16 new) \u2014 coverage 92.99% \u2265 80% \u2705
- RUN_REAL_ADAPTER_TESTS=1: 6/6 \u2705 (Protocol conformance still
  holds after bones promotion; isinstance(LineRealAdapter,
  LineAdapter) + isinstance(LineMockAdapter, LineAdapter) both
  return True)
- ruff + mypy strict: clean

## Out of scope

- strip_markdown leaks content from code spans (Backend warning) \u2014
  noted, follow-up
- _MD_ITALIC_STAR matches arithmetic (Backend warning) \u2014 noted,
  follow-up
- self-message filter default-off (Backend warning) \u2014 real wiring
  must set bot_user_id; documented
- chat_id / line_user_id reconciliation \u2014 currently same in DMs;
  groups deferred to when we add group support
- unsend event handling \u2014 1-migration, deferred (correctly noted in
  C3 fix)
The P0 fix on the previous commit added REPLY_TOKEN_TTL_SECONDS to
app.adapters.line.base, but left the same constant re-defined in
app.adapters.line.real (line 25-27). Behavior was correct (both
values are 60) but the dedup wasn't actually a dedup — real.py
imported nothing from base for the constant.

Removes the local definition in real.py and adds the constant to
the existing import block from base. No behavior change.
@choguun
choguun force-pushed the feat/line-hermes-takeaways branch from 0085dae to 30f665d Compare July 3, 2026 12:38
@choguun
choguun merged commit 80afc9b into main Jul 3, 2026
4 checks passed
choguun added a commit that referenced this pull request Jul 3, 2026
The AIDLC cycle for Month-1 MVP is closed. PR #1 (Month-1 MVP) and
PR #2 (hermes-agent#23197 takeaways) are both MERGED on main. Local
and remote feat/month-1-mvp deleted. Next: start a new AIDLC cycle.
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