feat: establish versioned UI translation ledger foundation - #929
seonghobae wants to merge 262 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough8개 로케일 번역 원장과 읽기 경로를 갱신했습니다. 캐시 검증, API 오류 매핑, 게시 보호, 롤백, Customer Master 초안, 문서와 검증 테스트를 확장했습니다. ChangesUI 번역 원장
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~120 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant AuthenticatedClient
participant FastAPI
participant translation_ledger
participant Valkey
participant PostgreSQL
AuthenticatedClient->>FastAPI: GET /api/translations/{screen_key}
FastAPI->>translation_ledger: read_translation_screen(...)
translation_ledger->>Valkey: Read exact-version cache candidate
Valkey-->>translation_ledger: Payload or cache miss
translation_ledger->>PostgreSQL: Validate evidence or fetch projection
PostgreSQL-->>translation_ledger: TranslationScreen
translation_ledger-->>FastAPI: Immutable translation projection
FastAPI-->>AuthenticatedClient: 200, 404, 409, or 422
Merge Risk: ⚪ Minimal · up to The Customer Master migration now protects pre-existing drafts from adoption or rollback deletion, and the documentation check scopes PR 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.10% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 261 functions across 33 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Valid finding: the explicit-version Valkey fast path can make a structurally valid but incomplete cache payload authoritative. _decode_cached_screen() validates identity and nonblank values, but it has no independent knowledge of the published resource's required key set. A payload for the exact product/screen/version/locale containing only title therefore bypasses PostgreSQL and returns an incomplete screen, contradicting this PR's fail-closed completeness contract and its statement that PostgreSQL remains authoritative. Add a realistic RED for a correct-identity/partial-key cache payload, then make cache acceptance depend on authoritative published key-set evidence (without cross-locale fallback).
|
Review finding on current head |
|
Review finding on |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Application/DB identity semantics are not actually aligned yet. ADR 0362 says leading/trailing-whitespace product_key and screen_key forms are rejected consistently at PostgreSQL and the application boundary, and migration 0246 rejects them with btrim equality. _validate_identity_segment() instead strips caller input and accepts it, so lineageweave aliases lineageweave before DB/cache lookup. That silently normalizes an ambiguous external identity while the persisted aggregate forbids that spelling. Add a RED covering padded product/screen inputs failing before I/O, then make the application boundary reject rather than normalize.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
The new real-PostgreSQL translation test adds fresh direct psycopg2 / psycopg2.errors reachability even though repository issue #910 requires every direct psycopg2 caller to be migrated and #911 is removing that dependency path. This branch should not add a new caller that would have to be deleted during convergence. Keep the test independent of mutable #911 source by using the already-owned asyncpg runtime boundary for its throwaway PostgreSQL setup/assertions, and lock that with a RED before refactoring.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Valid finding on exact head 8b01c7c82cf3f7923868c38a5c7f4fd9379d966d: explicit-version reads acquire an asyncpg connection, fetch the required-key set, and then await Valkey get() before releasing that PostgreSQL lease. Because published resource/key sets are immutable, the DB lease is not needed while waiting on cache I/O. A slow or wedged cache can therefore pin one PostgreSQL pool slot per request and create buyer-visible pool starvation/latency even though Valkey is explicitly non-authoritative. Repair should release the PostgreSQL connection immediately after the key-set query, perform the cache read outside the pool lease, and reacquire only on cache miss/failure. Add a regression that observes lease state at cache-read time; do not weaken fail-closed key-set validation.
|
Review finding on exact head |
|
Owner-path handoff from Similar VOC #1126 exact |
|
Concrete owner successor opened for the #1126 Similar VOC screen-copy handoff: #1127, Draft, stacked directly on #929 exact |
|
Similar VOC translation successor #1127 advanced by ordinary commits to exact |
|
#1127 owner-path handoff update: current exact head is |
|
#1127 owner-path correction: current exact head is |
|
#1127 owner handoff update: Similar VOC screen-copy successor is now Draft at exact |
|
#1127 translation-owner successor advanced to exact |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review: the Customer Master ownership rollback previously erased a valid retired one-time-seed receipt, contradicting the no-resurrection lifecycle already enforced on ordinary deletion/startup. RED 92ac572f... now requires real PostgreSQL rollback to fail closed and preserve retired / resource_id=NULL; causal fix 314be833... withdraws only unowned pending|blocked reservations and refuses to destroy retired history. Direct children #932 and #1127 were converged by ordinary two-parent commits without force-push or child-delta duplication. Exact-head Tests are Draft-policy skipped and Security/SAST/CodeQL are not yet accepting, so this is source-level evidence only, not approval or promotion authority.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head evidence repair on 156bd71dc53cfb6315f3396437651eac88649596: the September 21 finding remained valid. The existing 37×8 completeness/publishability fixture applied 0248 without the production predecessor 0247_z_customer_master_translation_seed_ownership.sql, replay guard, or the lineageweave.migration_file provenance used by docker/postgres-init/migrate.sh, so it proved a different migration path. Realistic RED e74d6e8a139d0e7da5b3a3e8186f869790bc5d64 requires an owned 0248 receipt on that evidence path. Causal evidence fix 156bd71dc53cfb6315f3396437651eac88649596 adds a real-PostgreSQL canonical sorted path with runner-equivalent provenance and verifies the owned receipt, one 37×8 draft, reviewed-copy replay preservation, publication, published rollback refusal, and exact-owned draft rollback. This changes test/evidence only; no translation runtime, seed bytes, publication authority, or rollback implementation changed. Exact-head Tests remain Draft-policy skipped, so this is source-level evidence and not approval or promotion authority. Direct children #932 and #1127 were immediately converged by ordinary two-parent commits with the parent test blob adopted unchanged; no force push or destructive rebase.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head recovery review: ui_translation_seed_ownership carries durable one-time-seed state (retired is the no-resurrection fact), but the existing TRUNCATE guard covered only ui_translation_resource/key/text. Because the ownership table references the root, root TRUNCATE ... CASCADE can include and erase that history while no resource is published, bypassing row-level retirement/rollback guards. RED 2a87b299... pins the real-PostgreSQL failure mode. Causal fix 527d7e196... adds a separately ordered ownership-table BEFORE TRUNCATE fail-closed guard plus paired rollback and moves the production-path fixture through 0247_z -> 0247_za -> 0247_zz -> 0248. Direct children #932 and #1127 were non-force two-parent converged. Exact-head Tests/PROV-O/Ontology are Draft-policy skipped; Security/SAST/CodeQL are still nonterminal, so this is source-level evidence only and not approval/promotion authority.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh recovery review found a second destructive gap in the new seed-ownership TRUNCATE protection. The forward guard was valid, but rollback/0247_za_ui_translation_seed_ownership_truncate_guard.sql removed it in a separate committed step before rollback/0247_z_customer_master_translation_seed_ownership.sql validated whether retirement history could actually be removed. With a valid retired / resource_id=NULL receipt, the second rollback correctly fails closed; the predecessor therefore left the ownership table in place but unprotected, reopening TRUNCATE ui_translation_resource CASCADE as a way to erase no-resurrection history.
Real-PostgreSQL RED 84a7b1fc7af8bd0ac1a00c03755a14568deee192 extends the retirement scenario through 0247_za rollback -> expected 0247_z refusal -> root TRUNCATE and requires the guard/receipt to survive the failed recovery. Causal repair 787aac211cafeb90e2459538c1877b2ac1ac2c09 makes the 0247_za rollback non-destructive; final exact head 1219fe4b2d9dcc3388c286aa9fdeb4fdac60c566 moves trigger/function removal into the successful 0247_z ownership-table rollback transaction, after all resource/receipt preconditions pass. Any exception rolls the whole transaction back and leaves the TRUNCATE guard live.
This is source-level recovery evidence only. Exact-head Tests are Draft-policy skipped; Security/SAST/CodeQL are currently queued, and no qualifying independent current-head approval exists. Keep Draft; do not Ready-cycle, auto-merge, blind-rerun, or resolve the long-running ownership thread until unchanged-head PostgreSQL/full-suite and required hosted acceptance are real GREEN.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review: the ownership replay path locked ui_translation_seed_ownership and then the Customer Master root, while ordinary DELETE acquires the root row before its BEFORE DELETE trigger retires the ownership receipt. That is a real lock-order inversion. RED b3df3853... stages replay and DELETE behind an ownership-row holder so the predecessor reaches the deadlock cycle; causal fix 8153ece6... removes the replay-side root row lock and keeps the ownership row as the serialization point. This is source-level evidence only until exact-head real-PostgreSQL/full-suite and required hosted checks are actually GREEN. Keep Draft; this comment is not approval.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review found a buyer-visible language defect in the Customer Master hint action. The route resolves an opaque source customer identifier to a corroborated organization candidate before any binding, but the 0248 draft translated Resolve as generic finalization/solving/parsing terms (확정, 解決, 解析, Resolver, Auflösen, Résoudre). RED 44c6ccc4... fixes the reviewed intent as organization identification/verification across the seven translated locales; repair f076fa46... updates only presentation copy and preserves the English contract key/value, resolver semantics, auth, ontology ownership, publication state, and consumer source. This is COMMENT only, not independent approval. Keep Draft until exact-head executable checks and qualifying independent language/product review exist.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if task is not None and not task.done(): | ||
| task.cancel() | ||
| with suppress(asyncio.CancelledError, asyncpg.PostgresError): | ||
| await task |
This branch has not been deployed
1 participant
Scope / owner boundary
PostgreSQL-authoritative versioned UI translation ledger plus authenticated translation API. LineageWeave owns product UI copy/presentation read-model behavior only; ontology/concept labels and semantic truth remain canonical-owner concerns. Customer Master copy remains a PostgreSQL draft.
Current exact authority — 2026-09-25
main@83eba56149eb802cd63642c507c324c9976ec78ef076fa46976b2f68942a6e066cdbcf8895ba16f46e5ba4773e5f21bafd7437986ec4b43bf5907639, 43 ahead / 0 behind from this exact parent4cfe2fe57352590f85010df6f2555c4b0e4eca4a, 75 ahead / 0 behind from this exact parentFresh buyer-language finding: generic “resolve” copy obscured the actual governed action
The Customer Master button calls the hint-resolution path for an opaque source customer identifier. That path may bind a customer only after the proposed organization name is externally corroborated; it is not a generic problem-solving action and it is not a user confirmation/finalization step.
The draft nevertheless rendered the
Resolve/Resolving...action as Korean확정, Japanese解決, Chinese解析, SpanishResolver, GermanAuflösen, and FrenchRésoudre. Those words respectively lean toward finalizing, solving, parsing, or technical dissolution rather than naming the buyer task. This is especially risky beside copy that says source identifiers are hints only and must be resolved before binding a customer.44c6ccc446e891f36e300efb90397847b2412136: adds an exact eight-locale contract requiring the hint action to name organization identification/verification rather than generic solving/finalization; the predecessor 0248 draft fails that contract;f076fa46976b2f68942a6e066cdbcf8895ba16f4: keeps the English source key/valueResolve/Resolving...for contract compatibility, while the seven translated presentation values now explicitly name organization identification or verification (조직 식별,組織を特定,识别组织,Xác minh tổ chức,Identificar organización,Organisation identifizieren,Identifier l’organisation) and corresponding in-progress states;This is an owner-side product-language review and a regression contract, not qualifying independent language approval. External/current-head language/product review remains required before publication.
Retained transaction/recovery evidence
The seed-owner replay lock-order RED
b3df3853f38a7c5019dac0994c9dddcd80ad0750and repair8153ece6be2a7671cef507a04c7da08145881826remain in the ancestry: ownership receipt is the lifecycle serialization point and replay-side resource identity is a plain MVCC read, eliminating theownership -> rootversus ordinary-deleteroot -> ownershipdeadlock edge. Prior TRUNCATE no-resurrection hardening, production-path seed evidence, retirement preservation, and rollback atomicity also remain in force.Descendant convergence
Parent movement was not left as a note. #1127 and #932 adopted
f076fa46...through ordinary non-force two-parent convergence. The parent changes touch only the Customer Master 0248 draft copy and its buyer-copy regression; #1127 retains its Similar VOC owner delta, and #932 retains its Customer Master consumer/UI delta. No force push, destructive rebase, or parent-source copy was used.Exact-head hosted state
On
f076fa46...:36023533720: skipped by Draft policy36023533821: skipped36023533825: skipped36023533676: queued on last exact-head read36023533781: queued on last exact-head read36023533855: queued on last exact-head readTherefore
44c6ccc4... -> f076fa46...is source-level RED→repair evidence only. No exact-head executable full-suite/PostgreSQL GREEN, complete Security/SAST/CodeQL/Strix/model acceptance, qualifying independent language/product approval, immutable publication, consumer/browser acceptance, qualifying current-head approval, protected merge, or release evidence is established.Promotion gate
Keep Draft. Do not Ready-cycle to materialize checks, enable auto-merge, add wake commits, synthesize status, blind-rerun, self-approve, dismiss reviews, weaken gates, force-push, destructively rebase, publish prematurely, merge, or release.