Cloonar/deckle
3
0
Fork 0
Code Issues 13 Pull requests Projects Releases 63 Packages Wiki Activity Actions

fix(pageops): translate-plan and translate decide through one owner #281

Merged
dominik.polakovics merged 2 commits from afk/275 into main 2026-10-14 18:38:44 +02:00
Conversation 2 Commits 2 Files changed 9 +401 -103
dominik.polakovics commented 2026-10-14 09:19:09 +02:00
Owner
Copy link

Closes #275

What

  • New read-only owner planTranslation(root, page, lang, subpages) in internal/pageops/translation.go. It decides the page's own refusal (in the translate's order: translation.exists → translation.addressTaken → translation.sourceParse) and, with subpages, which subpages are created and which are skipped (source does not parse, address taken). It needs no mode, label or schemas, and it returns the sources it read and parsed so Translate does not read them again.
  • TranslatePlan returns len(create) - 1 from the owner. It now answers 409 translation.addressTaken and 422 translation.sourceParse just as the translate does, and counts only subpages the translate will create.
  • Translate validates the mode, calls the owner, then only renders (renderTranslation, the former translationBytes without the parse step) and writes. The write and rollback code is unchanged.
  • The tree is built once per decision. The new translationAddressTakenIn(tree, …) is now the body of translationAddressTaken, so the address rule still lives in one place and is unchanged.

Decisions (as in the issue)

  • The plan answer stays {subpages: number}.
  • The owner parses every candidate page.
  • A subpage whose rendering fails is now an error, not a skip. This covers l10n.TranslateDocument, the front matter and the serializer. The translate fails with that error (the same 400/422 the page itself would get) and writes nothing. skipped lists only the two documented reasons. Checked: web/src parses skipped but never displays it.

Docs and comments

  • docs/api.md:
    • GET /api/page/translate-plan: the "neither checks the address … nor parses the source" sentence and the "skipped subpages are counted" clause are gone. Its errors now list 409 translation.addressTaken and 422 translation.sourceParse.
    • POST /api/page/translate: says a subpage render failure fails the translate.
  • These comments now name the actual shared set: handlePageTranslatePlan / handlePageTranslate, TranslatePlan / Translate, fetchTranslatePlan, TranslateDialog.
  • No skill or scaffold/site/AGENTS.md change: none of them describes this route (as the issue notes).

Tests

  • internal/pageops/translation_test.go:
    • The plan and the translate refuse with the same kind, code and args for the sibling slug collision and for an unparsable page.
    • With one subpage whose address is taken and one whose source does not parse, the plan's count equals len(Created) - 1.
    • A subpage render failure (a label with quotes makes a param value the serializer refuses) fails the translate with translation.serialize and writes and stamps nothing.
  • internal/pageops/codes_test.go: TestTranslationErrorCodes covers the plan's translation.addressTaken.
  • internal/server/translation_pages_test.go: the plan route answers the same status and coded body as the translate route for 409 translation.addressTaken and 422 translation.sourceParse.
  • go vet ./..., go test ./..., gofmt and the web suite (npm test, 3213 tests) are all green.

🤖 Generated with Claude Code

Closes #275 ## What - New read-only owner `planTranslation(root, page, lang, subpages)` in `internal/pageops/translation.go`. It decides the page's own refusal (in the translate's order: `translation.exists` → `translation.addressTaken` → `translation.sourceParse`) and, with subpages, which subpages are created and which are skipped (source does not parse, address taken). It needs no mode, label or schemas, and it returns the sources it read and parsed so `Translate` does not read them again. - `TranslatePlan` returns `len(create) - 1` from the owner. It now answers `409 translation.addressTaken` and `422 translation.sourceParse` just as the translate does, and counts only subpages the translate will create. - `Translate` validates the mode, calls the owner, then only renders (`renderTranslation`, the former `translationBytes` without the parse step) and writes. The write and rollback code is unchanged. - The tree is built once per decision. The new `translationAddressTakenIn(tree, …)` is now the body of `translationAddressTaken`, so the address rule still lives in one place and is unchanged. ## Decisions (as in the issue) - The plan answer stays `{subpages: number}`. - The owner parses every candidate page. - **A subpage whose rendering fails is now an error, not a skip.** This covers `l10n.TranslateDocument`, the front matter and the serializer. The translate fails with that error (the same 400/422 the page itself would get) and writes nothing. `skipped` lists only the two documented reasons. Checked: `web/src` parses `skipped` but never displays it. ## Docs and comments - `docs/api.md`: - `GET /api/page/translate-plan`: the "neither checks the address … nor parses the source" sentence and the "skipped subpages are counted" clause are gone. Its errors now list `409 translation.addressTaken` and `422 translation.sourceParse`. - `POST /api/page/translate`: says a subpage render failure fails the translate. - These comments now name the actual shared set: `handlePageTranslatePlan` / `handlePageTranslate`, `TranslatePlan` / `Translate`, `fetchTranslatePlan`, `TranslateDialog`. - No skill or `scaffold/site/AGENTS.md` change: none of them describes this route (as the issue notes). ## Tests - `internal/pageops/translation_test.go`: - The plan and the translate refuse with the same kind, code and args for the sibling slug collision and for an unparsable page. - With one subpage whose address is taken and one whose source does not parse, the plan's count equals `len(Created) - 1`. - A subpage render failure (a label with quotes makes a param value the serializer refuses) fails the translate with `translation.serialize` and writes and stamps nothing. - `internal/pageops/codes_test.go`: `TestTranslationErrorCodes` covers the plan's `translation.addressTaken`. - `internal/server/translation_pages_test.go`: the plan route answers the same status and coded body as the translate route for `409 translation.addressTaken` and `422 translation.sourceParse`. - `go vet ./...`, `go test ./...`, `gofmt` and the web suite (`npm test`, 3213 tests) are all green. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(pageops): translate-plan and translate decide through one owner
All checks were successful
ci / build-and-test (pull_request) Successful in 3m9s
Details
ci / release (pull_request) Has been skipped
Details
f8331bbb42 
planTranslation in internal/pageops now owns "can this page be translated
into lang, and which subpages will be created". TranslatePlan reports from
its result and Translate executes it, so the plan refuses exactly what the
translate refuses (translation.exists, translation.addressTaken,
translation.sourceParse, in that order) and counts only the subpages the
translate will create. The page tree is built once per decision.

A subpage whose translation fails to render (l10n.TranslateDocument, its
front matter, the serializer) now fails the translate with its own error
instead of being silently listed in skipped; skipped keeps the two
documented reasons only.

docs/api.md, the handler comments and the client comments now name the
actual shared refusal set.

Closes #275

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dominik.polakovics commented 2026-10-14 09:23:58 +02:00
Author
Owner
Copy link

[autoland] verdict: pass

Verdict: CONCERNS (non-blocking). Approved, not auto-merged.

Signal relied on: labctl pr checks 281 aggregate success (ci / build-and-test green, ci / release skipped) at head f8331bb. That run covers go vet/test, the docs drift tests and the route-doc test, so I did not re-run them. The head is current with main (merge-base bf70703), so there is no conflict.

Conventions: the title is a Conventional Commit (fix(pageops): …) and the body has Closes #275. The diff stays within #275's scope (pageops owner, handler, docs/api.md, comments-only web changes). Skills and AGENTS.md are correctly left alone because the issue's out-of-scope list excludes them.

Every acceptance criterion is met:

  • planTranslation (internal/pageops/translation.go) is the single owner, and both TranslatePlan and Translate call it.
  • Refusal order is unchanged: exists, then addressTaken, then sourceParse.
  • The tree is built once, and translationAddressTakenIn keeps the address rule in one place.
  • A subpage render failure now fails the translate before any write. TestTranslateSubpageRenderFailureFails shows nothing is written or stamped.
  • Plan-equals-translate is tested at both the pageops level and the route level.
  • TestTranslationErrorCodes covers the plan's translation.addressTaken.

Concern (docs drift, non-blocking):

  • docs/api.md:1525 (### GET /api/page/translate-plan errors) is missing 500 page.read {detail}. The plan route can now return it: planTranslation reads the page and every candidate subpage through readTranslationSource (internal/pageops/translation.go:231, :243 → :266 errInternal(apimsg.PageRead)), and before this PR the plan never read a source. The translate's list includes page.read, and the plan's list otherwise names all its 500s (page.readLanguages, page.buildTree). Fix: add page.read to the plan's 500 group.
[autoland] verdict: pass Verdict: CONCERNS (non-blocking). Approved, not auto-merged. Signal relied on: `labctl pr checks 281` aggregate `success` (ci / build-and-test green, ci / release skipped) at head f8331bb. That run covers go vet/test, the docs drift tests and the route-doc test, so I did not re-run them. The head is current with main (merge-base bf70703), so there is no conflict. Conventions: the title is a Conventional Commit (`fix(pageops): …`) and the body has `Closes #275`. The diff stays within #275's scope (pageops owner, handler, docs/api.md, comments-only web changes). Skills and AGENTS.md are correctly left alone because the issue's out-of-scope list excludes them. Every acceptance criterion is met: - `planTranslation` (internal/pageops/translation.go) is the single owner, and both `TranslatePlan` and `Translate` call it. - Refusal order is unchanged: exists, then addressTaken, then sourceParse. - The tree is built once, and `translationAddressTakenIn` keeps the address rule in one place. - A subpage render failure now fails the translate before any write. TestTranslateSubpageRenderFailureFails shows nothing is written or stamped. - Plan-equals-translate is tested at both the pageops level and the route level. - TestTranslationErrorCodes covers the plan's `translation.addressTaken`. Concern (docs drift, non-blocking): - docs/api.md:1525 (`### GET /api/page/translate-plan` errors) is missing `500` `page.read` `{detail}`. The plan route can now return it: `planTranslation` reads the page and every candidate subpage through `readTranslationSource` (internal/pageops/translation.go:231, :243 → :266 `errInternal(apimsg.PageRead)`), and before this PR the plan never read a source. The translate's list includes `page.read`, and the plan's list otherwise names all its 500s (`page.readLanguages`, `page.buildTree`). Fix: add `page.read` to the plan's `500` group.
docs(api): translate-plan lists 500 page.read
All checks were successful
ci / build-and-test (pull_request) Successful in 3m7s
Details
ci / release (pull_request) Has been skipped
Details
fddd5769c6 
The plan now reads the page and every candidate subpage through
readTranslationSource, so it can answer 500 page.read like the translate.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dominik.polakovics commented 2026-10-14 18:32:41 +02:00
Author
Owner
Copy link

Docs drift fixed in fddd576: GET /api/page/translate-plan now lists 500 page.read alongside page.readLanguages and page.buildTree, since planTranslation reads the page and every candidate subpage through readTranslationSource. go test ./docs/... ./internal/server/... is green.

Docs drift fixed in fddd576: `GET /api/page/translate-plan` now lists `500` `page.read` alongside `page.readLanguages` and `page.buildTree`, since `planTranslation` reads the page and every candidate subpage through `readTranslationSource`. `go test ./docs/... ./internal/server/...` is green.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is broken

  
enhancement
New feature or improvement

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

No labels
bug
enhancement
needs-triage
ready-for-agent
ready-for-human
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Cloonar/deckle!281
No description provided.