feat: canonical view for translation-suffix bodies (translation-bot-compat) - #455
Conversation
Add internal/model/translation.go with the single-literal divider allowlist, preview-mirroring fence guard (stop-on-unclosed-fence), single-divider rule, StripTranslationSuffix, the strip-only CanonicalView, CanonicalBodyEqual, TranslationDividerLine, and CanonicalTitle. Table tests cover the Design test 1 cases including two-divider fail-closed, lone-divider edge, CRLF, and fenced dividers. Design higress-group#453 D1/D3/D4/D6/D7. Signed-off-by: 澄潭 <zty98751@alibaba-inc.com>
…54002) ParseTypedComment and FindMarker consume the composed view (preview.SemanticView of CanonicalView); LogicalBody and TypedSectionList canonicalize internally with the strip-only view, preserving byte-identical passthrough for untranslated bodies. validatePhasePredecessor, change.parseMarker/exactPredecessor, and canonicalProposalContribution scan the canonical view directly. Reference title equality compares through CanonicalTitle so a machine-translation ` || ` suffix is not drift. Tests cover parser equivalence, preview passthrough, issue-body consumers (degraded copies, genuine duplicates, unclosed-fence fail-closed), and titles. Design higress-group#453 D1(a)-(c), D3a, D6. Signed-off-by: 澄潭 <zty98751@alibaba-inc.com>
…54003)
RepresentationDigest hashes the strip-only canonical view ("digest of the
stripped raw body, no preview masking"); the exact-bytes contract test is
replaced with suffix-stability, passthrough, and preview-source-visibility
tests. Exact-byte postconditions at engine.go and link.go become canonical
equality with canonical-digest messages (exact->canonical wording), so a
translation suffix appended after issue-spec's own write is not a conflict
while canonical drift still aborts. removeSemanticAuthority bounds its scan
at the divider line: suffix matches are excluded, within-prefix duplicates
still fail ErrAmbiguous, and raw-line index arithmetic is unchanged. The
generated workflow skill documents suffix tolerance and the never-author-
the-divider rule (byte budget raised accordingly). Tests cover write-path
guards, relationship boundary, graph dedupe, runner surfaces, and the
edited-ANSWER fail-closed residual.
Design higress-group#453 D2, D5, D8, D9, D10.
Signed-off-by: 澄潭 <zty98751@alibaba-inc.com>
johnlanni
left a comment
There was a problem hiding this comment.
Independent Review — qoder-k3 (Kimi-K3, exact head 280b607)
(GitHub forbids approving your own PR, so this is posted as a COMMENT; treat it as the review verdict.)
Verdict: APPROVE — D1–D11 faithfully implemented; all declared deviations verified as bounded; fail-closed behavior preserved everywhere the canonical view does not apply. No P0, no blocking P1.
Checklist verification highlights
- translation.go: allowlist single exact literal; fence opener/closer byte-semantics identical to
preview.parseOpener/isCloser(≤3 leading spaces, ≥3 run, backtick-in-info rejection, stop-on-unclosed-fence); single-divider fail-closed rule; CRLF handled per preview splitLines. - Integration completeness: every design-named consumer converted —
marker.go:28,typed_comment.go:282,validation.go:39(LogicalBody),links.go:139(TypedSectionList),issue.go:333,locator.go:154,180,service.go:421,bindings/service.go:647-652,RepresentationDigest(typed_comment.go:26),engine.go:570,588,638,653,link.go:313,335,347,mutation.go:336. Remaining raw paths are same-suffix comparisons (transition.go) or fail-closed (see P2-2). - Untranslated passthrough: byte-identical when no qualifying divider; pinned by tests incl. preview-body passthrough and the retained exact-bytes digest contract for no-suffix bodies.
- D8: all 7 sites use
CanonicalBodyEqual; digest-equality implies view-equality, so no identical-digest-mismatch message is possible; real conflicts still reported with differing digests (pinned byTestReconcileStillConflictsOnCanonicalDriftAfterWrite); version-CAS residual documented atengine.go:561-563. - D9: raw index arithmetic unchanged; suffix excluded; within-prefix duplicates still ErrAmbiguous; suffix survives mutation writes.
- Digest re-basing: all consumers route through the same canonical function — intra-run consistent; pre-change sealed digests of untranslated bodies unchanged; translated-body flows were already broken pre-change. Migration acceptable.
P2 findings (non-blocking)
- Digest-stability boundary narrower than deviation-1 wording:
digest(original) == digest(original+suffix)holds only when the original ends in exactly one LF (translation.go:28-33). Zero or ≥2 trailing newlines → mismatch at sealed-digest sites (engine.go:622,726-727,752-753,native_answers.go:196,link.go:335) — all fail closed, never corrupt. Recommend documenting the exactly-one-LF boundary and pinning a fail-closed test for a no-trailing-newline original. - Scope follow-ups (fail-closed, same class as deviation 4):
parseProjectionMarkers(projection.go:555) and specialized marker parsers (rationale.go:28,finding.go:52,140,code_change_rationale.go:229) still scan raw bodies. Recommend one tracking issue bundling them withObserveAcceptedReceiptAuthority. - Perf nit: add a
strings.Contains(body, divider)fast path before split/join on the dominant no-divider path. - Unpinned adversarial cases (verified correct by reading): blockquoted divider, mixed fence chars, divider-as-last-line, 4-space-indented divider (currently qualifies per "optional surrounding whitespace" — intentional).
Reviewed HEAD: 280b607b6570536c5346742a973bdad36dacd801. Worktree clean throughout; static analysis only (in-session test execution was unavailable — suite green per author and CI).
higress-group#455) P2-1: Document the digest-stability boundary in StripTranslationSuffix (exactly-one-LF originals keep their digest across a suffix; zero or 2+ trailing LFs fail closed because the join byte count is not recoverable). Full stability was rejected: always normalizing trailing newlines would change digests of untranslated bodies and break sealed-digest continuity. Tests pin both directions plus the fail-closed link.go:335 guard. P2-2: Convert the remaining specialized marker parsers to the canonical view, matching each function's existing masking: composed view for parseProjectionMarkers and ObserveAcceptedReceiptAuthority (both already masked via SemanticView), strip-only for FindRationaleMarker, FindFindingMarker, FindFindingReplyMarker, and FindCodeChangeRationaleMarker (raw today). Untranslated behavior is byte-identical. Focused tests per site: translated body passes, genuine duplicates still surface/fail. P2-3: Fast path in StripTranslationSuffix returning (body, false) before any scanning when the allowlist literal is absent (single entry today); allocation test proves the no-divider path is allocation-free. P2-4: Pin adversarial table cases: blockquoted divider (not qualifying), mixed fence chars (``` closed by ~~~ stays open, no strip), divider as last line without trailing newline (strips), four-space indented divider (qualifies, intentional per optional surrounding whitespace). Signed-off-by: 澄潭 <zty98751@alibaba-inc.com>
Re-review Round 2 — qoder-k3 (exact head 53fd5d7)Verdict: APPROVE — all four P2 responses implemented as described, correctly bounded, pinned by non-tautological tests.
Caveat: static review per session constraints; suite green per implementer report and CI (96 packages ok). Reviewed HEAD: 53fd5d7 |
Summary
lizheming/github-translate-action), so issue-spec parsing, validation, digests, and write-path guards tolerate translated bodies without rewriting them.translation-bot-compatper the issue-spec governance flow:issue-api-compatibility)Why
The translate action appends a translated copy after an exact divider line, duplicating issue-spec markers,
##sections, and canonical JSON sections. Live breakage:issue create designpredecessor validation (found 2markers), exactly-once## Choice Model/## Answer/## Questionparsing, duplicate relationship edges, and digest churn on every bot re-translation (observed on higress-group/HiRoute #1/#9).What (per TASK)
internal/model/translation.go: divider allowlist (single exact literal, line-anchored), fence-state guard mirroringinternal/preview(including stop-on-unclosed-fence), single-divider rule (strip only when exactly one qualifying divider exists; two → fail-closed),StripTranslationSuffix/CanonicalView. 17-case table test.FindMarker/ParseTypedComment(composed view),LogicalBody+TypedSectionList(strip-only, byte-identical passthrough for untranslated preview bodies),validatePhasePredecessor,locator.parseMarker/exactPredecessor,canonicalProposalContribution;CanonicalTitle||normalization at exact-title comparisons.RepresentationDigestre-based on the strip-only view (doc contract: "digest of the stripped raw body, no preview masking"; preview-source-only edits stay digest-visible); write-path exact-byte guards atengine.goandlink.gobecome canonical comparisons with canonical-digest messages (suffix-only drift passes, canonical drift still conflicts); relationship semantic scans bounded at the divider (within-prefix duplicates stillErrAmbiguous); generated skill guidance note (never hand-strip, never author the divider literal outside fences).Test plan
go build ./...go test -p 1 ./...— 96 packages, 0 failurestranslation_test.go,translation_parsing_test.go,issue_translation_test.go,locator_translation_test.go,engine_translation_test.go,link_translation_test.go,mutation_translation_test.go,artifacts_translation_test.go,translation_workflow_test.go,title_test.go,model_test.go)Design deviations (documented in the implementation report): stripped results canonicalize trailing newline runs so
digest(original) == digest(original+suffix); D8 equality usesmodel.CanonicalBodyEqual(canonical views, join-byte tolerant); skill byte budget 12400→12800. Follow-up candidate noted:ObserveAcceptedReceiptAuthoritystill reads the raw semantic view (out of D1 scope).Closes #452