fix: render ADF document custom fields as plain text - #407
Conversation
FormatCustomFieldValue's map[string]any case did not detect Atlassian Document Format objects (type: 'doc'), causing ADF-backed custom fields (e.g. textarea, paragraph-rich) to render as '-' in 'issues get'. Add ADF detection: when the map has type='doc', marshal+unmarshal into ADFDocument and call ToPlainText() to extract readable text. Fixes reading of fields like 'Changelog Entry' that use ADF storage.
Automated PR ReviewReviewed commit: Summary
0 PR discussion threads considered. 0 summarized; 0 resolved. Completed in 1m 09s | ~$0.12 (est.) | claude-sonnet-4-6 | cr 0.4.161
Per-workstream usage
|
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR review completed with outcome: approved.
Add extension.Linkify to both mdParser and wikiParser so that bare URLs (e.g. https://example.com) in --description and --body text are automatically converted to clickable ADF hyperlinks.
When --description is used, the current issue is fetched first. Any mediaSingle/mediaGroup nodes (images, attachments) in the existing description are preserved and appended after the new markdown content. This prevents accidental deletion of embedded images when replacing text in a description.
monit-reviewer
left a comment
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: c466404a0876
Profile: pi-codex-monit-reviewer - Posting as: monit-reviewer
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 1 |
| policies:conventions | 0 |
| structure:repo-health | 0 |
| architecture:solid-reviewer-agnostic | 3 |
| harness-engineering:repo-health | 1 |
go:implementation-tests (1 finding)
Minor - shared/adf/convert.go:25
Linkification changes the generated ADF for bare URLs, but the conversion tests cover only explicit Markdown links. Add focused tests for bare URLs through both standard and wiki conversion paths, asserting a
linkmark and expectedhref, so this parser configuration change is protected from regression.
architecture:solid-reviewer-agnostic (3 findings)
Major - shared/adf/convert.go:25
U-T1/U-G1: enabling
Linkifychanges shared write-side conversion for every bare URL in both standard and wiki input, but it is unrelated to the stated read-side fix and has no focused test in this diff. Remove it from this PR; if intentional, ship it separately with directToDocument,ToDocumentWiki, andToJSONcontract tests.
Major - tools/jtk/internal/cmd/issues/update.go:140
U-S2/U-S3: the Cobra command now traverses concrete ADF nodes, selects media types, and defines merge ordering. That document transformation is core logic, not command orchestration, and the direct
atlassian-go/adfdependency leaks representation details into the shell. Move it to a pure API/internal-domain helper such asNewDescriptionPreservingMedia(text, existing)and unit-test the merge policy.
Major - tools/jtk/api/issues.go:426
ARCHITECTURE.md's Domain and Presenter/formatter layer rules, plus U-S1: this new branch deepens display normalization inside the importable
apidomain package by decoding raw API JSON directly into presentation text. Move custom-field display formatting, including ADF-to-plain-text conversion, into a presenter-owned pure formatter; keepapiresponsible for exposing raw or typed field values.
harness-engineering:repo-health (1 finding)
Major - tools/jtk/internal/cmd/issues/update.go:136
The new media-preservation invariant fails open: a
GetIssueerror silently falls back to writing a replacement ADF document, which can delete stored media during a transient read failure. Do not issue the description update when the preservation read fails; return a contextual error (or require an explicit destructive override). Add regression cases proving that a read failure makes no PUT and that a fetched media document retains its media nodes in the PUT payload.
Reviewer Coverage
go:implementation-tests— complete (broad); skipped: none; constraints: Read-only review; I inspected code and existing tests but could not execute the test suite.policies:conventions— complete (broad); inspected 2 assigned files (4 inspected across reviewers):tools/jtk/api/issues.go,tools/jtk/internal/cmd/issues/update.go; skipped: none; constraints: Shared CLI convention documents were not available in the review context; review used the repo-local conventions prompt.structure:repo-health— complete (broad); skipped: none; constraints: Static review limited to the assigned changed files; tests were not executed.architecture:solid-reviewer-agnostic— complete (broad); inspected 3 assigned files (4 inspected across reviewers):shared/adf/convert.go,tools/jtk/api/issues.go,tools/jtk/internal/cmd/issues/update.go; skipped: none; constraints: Read-only CR tools provide no command execution, so I inspected tests and manifests but did not run build, test, or lint. Review scope was the three assigned changed files, with repository guidance and nearby tests inspected for context. The external harness PRINCIPLES.md path is outside the disposable repository and unavailable through the permitted CR tools.harness-engineering:repo-health— complete (broad); skipped: none; constraints: Review was limited to the assigned changed files and read-only repository inspection; I did not execute tests.
Inspected files (4)
shared/adf/convert.gotools/jtk/api/issues.gotools/jtk/api/issues_extract_test.gotools/jtk/internal/cmd/issues/update.go
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 4m 30s | $0.28 | openai-codex/gpt-5.6-terra, openai-codex/gpt-5.6-sol | cr dev
| Field | Value |
|---|---|
| Model | openai-codex/gpt-5.6-terra, openai-codex/gpt-5.6-sol |
| Reviewers | go:implementation-tests, policies:conventions, structure:repo-health, architecture:solid-reviewer-agnostic, harness-engineering:repo-health |
| Engine | pi_rpc · openai-codex/gpt-5.6-terra, openai-codex/gpt-5.6-sol |
| Reviewed by | cr · monit-reviewer |
| Duration | 4m 30s wall · 8m 22s compute |
| Cost | $0.28 |
| Tokens | 29.5k in / 7.7k out |
Per-workstream usage
orchestrator-selection— openai-codex/gpt-5.6-terra- In: 5.4k
- Out: 521
- Cache read: 2.8k
- Cache create: 0
- Cost: $0.02
- Duration: 11s
go:implementation-tests— openai-codex/gpt-5.6-terra- In: 1.9k
- Out: 816
- Cache read: 17.9k
- Cache create: 0
- Cost: $0.02
- Duration: 1m 32s
policies:conventions— openai-codex/gpt-5.6-terra- In: 8.9k
- Out: 435
- Cache read: 0
- Cache create: 0
- Cost: $0.03
- Duration: 21s
structure:repo-health— openai-codex/gpt-5.6-terra- In: 2.1k
- Out: 817
- Cache read: 14.8k
- Cache create: 0
- Cost: $0.02
- Duration: 54s
architecture:solid-reviewer-agnostic— openai-codex/gpt-5.6-sol- In: 2.1k
- Out: 3.4k
- Cache read: 47.6k
- Cache create: 0
- Cost: $0.14
- Duration: 3m 52s
harness-engineering:repo-health— openai-codex/gpt-5.6-terra- In: 2.7k
- Out: 1.2k
- Cache read: 14.8k
- Cache create: 0
- Cost: $0.03
- Duration: 1m 16s
orchestrator-rollup— openai-codex/gpt-5.6-terra- In: 6.4k
- Out: 550
- Cache read: 0
- Cache create: 0
- Cost: $0.02
- Duration: 13s
|
|
||
| if description != "" { | ||
| fields["description"] = api.NewADFDocument(text.InterpretEscapes(description)) | ||
| // Fetch current issue to preserve media (images, attachments) in description |
There was a problem hiding this comment.
The new media-preservation invariant fails open: a GetIssue error silently falls back to writing a replacement ADF document, which can delete stored media during a transient read failure. Do not issue the description update when the preservation read fails; return a contextual error (or require an explicit destructive override). Add regression cases proving that a read failure makes no PUT and that a fetched media document retains its media nodes in the PUT payload.
Reply inline to this comment.
| var mdParser = goldmark.New( | ||
| goldmark.WithExtensions( | ||
| extension.Table, | ||
| extension.Linkify, |
There was a problem hiding this comment.
U-T1/U-G1: enabling Linkify changes shared write-side conversion for every bare URL in both standard and wiki input, but it is unrelated to the stated read-side fix and has no focused test in this diff. Remove it from this PR; if intentional, ship it separately with direct ToDocument, ToDocumentWiki, and ToJSON contract tests.
Reply inline to this comment.
| return fmt.Sprintf("%g", val) | ||
| case map[string]any: | ||
| // Detect ADF documents (type: "doc") and extract plain text. | ||
| if t, _ := val["type"].(string); t == "doc" { |
There was a problem hiding this comment.
ARCHITECTURE.md's Domain and Presenter/formatter layer rules, plus U-S1: this new branch deepens display normalization inside the importable api domain package by decoding raw API JSON directly into presentation text. Move custom-field display formatting, including ADF-to-plain-text conversion, into a presenter-owned pure formatter; keep api responsible for exposing raw or typed field values.
Reply inline to this comment.
| existingIssue, fetchErr := client.GetIssue(ctx, issueKey) | ||
| if fetchErr == nil && existingIssue.Fields.Description != nil && existingIssue.Fields.Description.ADF != nil { | ||
| var mediaNodes []*adf.Node | ||
| for _, node := range existingIssue.Fields.Description.ADF.Content { |
There was a problem hiding this comment.
U-S2/U-S3: the Cobra command now traverses concrete ADF nodes, selects media types, and defines merge ordering. That document transformation is core logic, not command orchestration, and the direct atlassian-go/adf dependency leaks representation details into the shell. Move it to a pure API/internal-domain helper such as NewDescriptionPreservingMedia(text, existing) and unit-test the merge policy.
Reply inline to this comment.
| var mdParser = goldmark.New( | ||
| goldmark.WithExtensions( | ||
| extension.Table, | ||
| extension.Linkify, |
There was a problem hiding this comment.
Linkification changes the generated ADF for bare URLs, but the conversion tests cover only explicit Markdown links. Add focused tests for bare URLs through both standard and wiki conversion paths, asserting a link mark and expected href, so this parser configuration change is protected from regression.
Reply inline to this comment.
…cli-collective#407 extension.Linkify changes write-side markdown->ADF conversion for every bare URL in --description/--body text across both tools, which is unrelated to this PR's read-side ADF custom-field rendering fix. Restores shared/adf/convert.go to origin/main's parser configuration. Code review finding (MAJOR, shared/adf/convert.go:25).
d5b9c60 to
c466404
Compare
4 participants
Problem
jtk issues get --fields "Changelog Entry"(and any ADF-backed custom field) displays-instead of the actual content.Root cause: In
FormatCustomFieldValue, themap[string]anycase only looks for keysvalue,name,displayName. ADF documents havetype: "doc"and none of those — so they fall through toreturn "", which becomes"-"" viaOrDash`.This is the read side of ADF handling — #191 addressed the write path (auto-wrapping plaintext into ADF on update), but reading stored ADF values was still broken.
Fix
Detect ADF documents (maps with
type: "doc") in themap[string]anycase, marshal+unmarshal intoADFDocument, and call the existingToPlainText()method.Changes
tools/jtk/api/issues.go: Add ADF detection inFormatCustomFieldValue(7 lines)tools/jtk/api/issues_extract_test.go: Addadf_documenttest caseTesting
map[string]any{"type":"doc",...}→"Hello ADF\n"jtk issues get TIO-270 --fields "Changelog Entry"now shows the actual text