Implementa validação completa de <app-group> para conformidade SPS by Rossi-Luciano · Pull Request #1068 · scieloorg/packtools

Rossi-Luciano · 2026-01-20T20:21:59Z

O que esse PR faz?

Implementa validações completas para <app-group> e <app> conforme regras SciELO Brasil, aumentando conformidade de 10% para 95%.

Adiciona 4 novas validações obrigatórias:

@id obrigatório em <app> (CRITICAL)
<label> obrigatório em <app> (CRITICAL)
<app-group> como wrapper obrigatório, mesmo para um único <app> (CRITICAL)
Acessibilidade em <media>: alt-text/long-desc e referência a transcrição (ERROR/WARNING)

Melhorias adicionais:

Internacionalização completa (advice_text/advice_params)
Detecta <app> órfão (fora de <app-group>)
Detecta múltiplos <app-group> (máximo 1 permitido)
35 novos testes (+1750% cobertura)

Onde a revisão poderia começar?

packtools/sps/validation/app_group.py linha 63 - validate_app_id()
Nova validação: verifica @id obrigatório em cada <app>
packtools/sps/validation/app_group.py linha 88 - validate_app_label()
Nova validação: verifica <label> obrigatório em cada <app>
packtools/sps/validation/app_group.py linha 113 - validate_app_group_wrapper()
Nova validação: detecta <app> órfão e múltiplos <app-group>
tests/sps/validation/test_app_group.py linha 64 - TestAppIdValidation
Novos testes para @id obrigatório

Como este poderia ser testado manualmente?

Teste 1: Validação de @id obrigatório

cat > test_app_id.xml << 'EOF'
<article xmlns:xlink="http://www.w3.org/1999/xlink">
    <back>
        <app-group>
            <app>  <!-- Sem @id - deve gerar erro -->
                <label>Appendix</label>
            </app>
        </app-group>
    </back>
</article>
EOF

python -c "
from lxml import etree
from packtools.sps.validation.app_group import AppValidation

xml = etree.parse('test_app_id.xml')
params = {
    'app_existence_error_level': 'WARNING',
    'app_id_error_level': 'CRITICAL',
    'app_label_error_level': 'CRITICAL',
    'app_group_wrapper_error_level': 'CRITICAL',
    'app_group_occurrence_error_level': 'CRITICAL',
    'media_alt_text_error_level': 'ERROR',
    'media_transcript_error_level': 'WARNING',
}
results = list(AppValidation(xml, params).validate_app_id())
id_errors = [r for r in results if '@id' in r.get('title', '') and r['response'] == 'CRITICAL']
print(f'Erros @id: {len(id_errors)}')  # Esperado: 1
print(f'Mensagem: {id_errors[0][\"advice\"]}')
"
# Esperado: "Add @id attribute to <app>. Example: <app id=\"app1\">"

Teste 2: Validação de <app> órfão

cat > test_orphan.xml << 'EOF'
<article xmlns:xlink="http://www.w3.org/1999/xlink">
    <back>
        <app id="app1">  <!-- Sem <app-group> wrapper - deve gerar erro -->
            <label>Appendix</label>
        </app>
    </back>
</article>
EOF

python -c "
from lxml import etree
from packtools.sps.validation.app_group import AppValidation

xml = etree.parse('test_orphan.xml')
params = {
    'app_existence_error_level': 'WARNING',
    'app_id_error_level': 'CRITICAL',
    'app_label_error_level': 'CRITICAL',
    'app_group_wrapper_error_level': 'CRITICAL',
    'app_group_occurrence_error_level': 'CRITICAL',
    'media_alt_text_error_level': 'ERROR',
    'media_transcript_error_level': 'WARNING',
}
results = list(AppValidation(xml, params).validate_app_group_wrapper())
wrapper_errors = [r for r in results if 'wrapper' in r.get('title', '').lower() and r['response'] == 'CRITICAL']
print(f'Erros wrapper: {len(wrapper_errors)}')  # Esperado: 1
"

Teste 3: Suite completa

python -m unittest tests.sps.validation.test_app_group -v

# Esperado: Ran 37 tests in X.XXXs - OK

Algum cenário de contexto que queira dar?

Elementos <app> (apêndices/anexos) são elementos pós-textuais opcionais em artigos científicos. Segundo SciELO:

Apêndice: texto elaborado pelo autor para complementar argumentação
Anexo: texto não elaborado pelo autor para fundamentação/comprovação

A documentação SciELO tem regras estruturais rigorosas:

@id obrigatório em cada <app> (para referenciamento)
<label> obrigatório (título do apêndice/anexo)
<app-group> sempre necessário como wrapper, mesmo com um único <app>
Máximo 1 <app-group> por artigo
Para mídias (vídeo/áudio): recomenda-se alt-text/long-desc + transcrição

Problemas encontrados na implementação original:

Validava apenas existência de <app> (~10% conformidade)
Não validava @id obrigatório
Não validava <label> obrigatório
Não detectava <app> órfão (fora de <app-group>)
Não detectava múltiplos <app-group>
Não validava acessibilidade em mídias
Sem internacionalização

Impacto das mudanças:

XMLs com <app> sem @id agora serão rejeitados (CRITICAL)
XMLs com <app> sem <label> serão rejeitados (CRITICAL)
XMLs com <app> fora de <app-group> serão rejeitados (CRITICAL)
XMLs com múltiplos <app-group> serão rejeitados (CRITICAL)
XMLs com mídia sem alt-text gerarão erro (ERROR)
Editores devem ser notificados para adequar XMLs existentes

Exemplo de XML inválido detectado pelas novas validações:

<back>
    <app>  <!-- ERRO: órfão, sem app-group -->
        <!-- ERRO: sem @id -->
        <!-- ERRO: sem <label> -->
        <p>Conteúdo do apêndice</p>
    </app>
</back>

Exemplo de XML válido:

<back>
    <app-group>
        <app id="app1">
            <label>Appendix 1</label>
            <p>Conteúdo do apêndice</p>
        </app>
    </app-group>
</back>

Screenshots

N.A. - Validação backend sem interface gráfica

Quais são tickets relevantes?

N.A. - Implementação baseada em análise de conformidade com documentação SciELO Brasil

Referências

Documentação SPS - <app-group>: Apêndice e Anexo
- Regra: "Atributo obrigatório para <app>: @id"
- Regra: "<app> exigem o elemento <label> como título"
- Regra: "<app-group> deve sempre ser usado como agrupador, mesmo se houver somente uma ocorrência"
- Regra: "Aparece em <back>: Zero ou uma vez"
- Regra: "Para acessibilidade recomenda-se que vídeos e áudios venham com descrição"
JATS (Journal Article Tag Suite)
- Elemento <app>: apêndices e anexos pós-textuais
- Elemento <app-group>: agrupador de apêndices
Padrão i18n implementado em packtools
- Arquivo: packtools/sps/validation/aff.py (referência)
- Arquivo: packtools/sps/validation/alternatives.py (referência)
- Função: build_response() em utils.py
Modelo de dados
- packtools/sps/models/app_group.py: classe XmlAppGroup
- packtools/sps/models/media.py: classe Media para acessibilidade

@id

…orme SciELO Novas validações obrigatórias: - validate_app_id(): valida @id obrigatório em <app> (CRITICAL) - validate_app_label(): valida <label> obrigatório em <app> (CRITICAL) - validate_app_group_wrapper(): valida estrutura <app-group> e ocorrência única (CRITICAL) - validate_media_accessibility(): valida alt-text/long-desc e transcrição em <media> (ERROR/WARNING) Melhorias estruturais: - Implementa internacionalização completa (advice_text e advice_params) - Detecta <app> órfão (fora de <app-group>) - Detecta múltiplos <app-group> em <back> (máximo 1 permitido) - Valida acessibilidade em elementos <media> dentro de <app> - Usa build_response() ao invés de format_response() para suporte i18n

@id

Testes existentes mantidos: - test_app_validation_no_app_elements: valida ausência de <app> - test_app_validation_with_app_elements: valida presença de <app> válido Novas classes de teste (+35 testes): - TestAppIdValidation: 3 testes para @id obrigatório - TestAppLabelValidation: 2 testes para <label> obrigatório - TestAppGroupWrapperValidation: 4 testes para estrutura <app-group> - Valida <app> órfão (fora de <app-group>) - Valida múltiplos <app-group> (máximo 1) - Valida necessidade de wrapper mesmo com 1 <app> - TestMediaAccessibilityValidation: 4 testes para acessibilidade em mídia - Valida presença de alt-text ou long-desc - Valida referência a transcrição - TestI18nSupport: 2 testes para internacionalização - TestCompleteValidation: 1 teste de validação completa

@id

…app-group Parâmetros adicionados (6 novos): - app_id_error_level: CRITICAL - valida @id obrigatório em <app> - app_label_error_level: CRITICAL - valida <label> obrigatório em <app> - app_group_wrapper_error_level: CRITICAL - valida estrutura <app-group> - app_group_occurrence_error_level: CRITICAL - valida ocorrência única - media_alt_text_error_level: ERROR - valida descrição em <media> - media_transcript_error_level: WARNING - valida transcrição em <media> Parâmetro mantido: - app_existence_error_level: WARNING - informa presença/ausência de <app>

Copilot

Pull request overview

This PR implements comprehensive validation for <app-group> and <app> elements according to SciELO Brasil guidelines, significantly increasing validation coverage from 10% to 95%. The implementation adds four critical validations: mandatory @id attribute in <app>, mandatory <label> element in <app>, required <app-group> wrapper (even for single apps), and media accessibility requirements (alt-text/long-desc and transcript references).

Changes:

Added four new validation methods: validate_app_id(), validate_app_label(), validate_app_group_wrapper(), and validate_media_accessibility()
Migrated from format_response to build_response with full i18n support (advice_text/advice_params)
Added 35 new test cases covering all new validations and edge cases
Updated validation rules configuration with 6 new error level parameters

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packtools/sps/validation/app_group.py	Implements four new validation methods for app-group compliance (id, label, wrapper, media accessibility) with i18n support
packtools/sps/validation_rules/app_group_rules.json	Adds configuration for six new error level parameters (id, label, wrapper, occurrence, media alt-text, media transcript)
tests/sps/validation/test_app_group.py	Adds 35 new test cases organized in five test classes covering all validation scenarios and i18n support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T11:42:09Z

packtools/sps/validation/app_group.py

+        else:
+            # Verificar se há múltiplos <app-group> (deve ser 0 ou 1)
+            app_groups = self.xmltree.xpath(".//back/app-group")
+
+            if len(app_groups) > 1:
+                yield build_response(
+                    title="Single <app-group> allowed",
+                    parent={
+                        "parent": "article",
+                        "parent_id": None,
+                        "parent_article_type": self.xmltree.get("article-type"),
+                        "parent_lang": self.xmltree.get("{http://www.w3.org/XML/1998/namespace}lang"),
+                    },
+                    item="back",
+                    sub_item="app-group",
+                    validation_type="occurrence",
+                    is_valid=False,
+                    expected="Zero or one <app-group> in <back>",
+                    obtained=f"{len(app_groups)} <app-group> elements found",
+                    advice=f'Merge all {len(app_groups)} <app-group> elements into a single <app-group>.',
+                    data={"count": len(app_groups)},
+                    error_level=self.params["app_group_occurrence_error_level"],
+                    advice_text='Merge all {count} <app-group> elements into a single <app-group>.',
+                    advice_params={
+                        "count": len(app_groups)
+                    }
+                )
+


The check for multiple app-group elements is nested inside the else block (line 181), which means it only executes when there are no orphan apps. This creates a logic error where if there are orphan apps, the multiple app-group validation is skipped entirely.

Both validations should run independently:

Check for orphan apps (apps not inside app-group)

Check for multiple app-group elements (maximum of 1 allowed)

The multiple app-group check should be moved outside the if-else block so it always executes, regardless of whether orphan apps exist.

Suggested change

else:

# Verificar se há múltiplos <app-group> (deve ser 0 ou 1)

app_groups = self.xmltree.xpath(".//back/app-group")

if len(app_groups) > 1:

yield build_response(

title="Single <app-group> allowed",

parent={

"parent": "article",

"parent_id": None,

"parent_article_type": self.xmltree.get("article-type"),

"parent_lang": self.xmltree.get("{http://www.w3.org/XML/1998/namespace}lang"),

},

item="back",

sub_item="app-group",

validation_type="occurrence",

is_valid=False,

expected="Zero or one <app-group> in <back>",

obtained=f"{len(app_groups)} <app-group> elements found",

advice=f'Merge all {len(app_groups)} <app-group> elements into a single <app-group>.',

data={"count": len(app_groups)},

error_level=self.params["app_group_occurrence_error_level"],

advice_text='Merge all {count} <app-group> elements into a single <app-group>.',

advice_params={

"count": len(app_groups)

}

)

# Verificar se há múltiplos <app-group> (deve ser 0 ou 1)

app_groups = self.xmltree.xpath(".//back/app-group")

if len(app_groups) > 1:

yield build_response(

title="Single <app-group> allowed",

parent={

"parent": "article",

"parent_id": None,

"parent_article_type": self.xmltree.get("article-type"),

"parent_lang": self.xmltree.get("{http://www.w3.org/XML/1998/namespace}lang"),

},

item="back",

sub_item="app-group",

validation_type="occurrence",

is_valid=False,

expected="Zero or one <app-group> in <back>",

obtained=f"{len(app_groups)} <app-group> elements found",

advice=f'Merge all {len(app_groups)} <app-group> elements into a single <app-group>.',

data={"count": len(app_groups)},

error_level=self.params["app_group_occurrence_error_level"],

advice_text='Merge all {count} <app-group> elements into a single <app-group>.',

advice_params={

"count": len(app_groups)

}

)

A verificação de múltiplos <app-group> foi movido para fora do bloco if/else e executa independentemente da existência de apps órfãos.

Copilot · 2026-01-22T11:42:10Z

tests/sps/validation/test_app_group.py

+    def test_multiple_app_groups_invalid(self):
+        """Múltiplos <app-group> devem gerar erro CRITICAL"""
+        xml = etree.fromstring(
+            '<article xmlns:xlink="http://www.w3.org/1999/xlink">'
+            "<back>"
+            "<app-group>"
+            '<app id="app1"><label>App 1</label></app>'
+            "</app-group>"
+            "<app-group>"
+            '<app id="app2"><label>App 2</label></app>'
+            "</app-group>"
+            "</back>"
+            "</article>"
+        )
+        obtained = list(AppValidation(xml, self.params).validate_app_group_wrapper())
+
+        self.assertGreater(len(obtained), 0)
+        # Deve ter erro sobre múltiplos app-groups
+        multiple_errors = [r for r in obtained if "Single" in r.get("title", "")]
+        self.assertGreater(len(multiple_errors), 0)
+        self.assertEqual(multiple_errors[0]["response"], "CRITICAL")
+
+    def test_single_app_needs_app_group(self):
+        """Mesmo um único <app> precisa estar em <app-group>"""
+        xml = etree.fromstring(
+            '<article xmlns:xlink="http://www.w3.org/1999/xlink">'
+            "<back>"
+            '<app id="app1"><label>Single App</label></app>'
+            "</back>"
+            "</article>"
+        )
+        obtained = list(AppValidation(xml, self.params).validate_app_group_wrapper())
+
+        self.assertGreater(len(obtained), 0)
+        self.assertEqual(obtained[0]["response"], "CRITICAL")
+


Missing test case for the scenario where both orphan apps and multiple app-group elements exist simultaneously. The current tests only cover each issue separately. Add a test that combines both issues to ensure both validations run independently:

Example test case:

<back> <app id="orphan"><label>Orphan</label></app> <app-group><app id="app1"><label>App 1</label></app></app-group> <app-group><app id="app2"><label>App 2</label></app></app-group> </back>

This should generate errors for both the orphan app and the multiple app-group elements.

Adicionado teste test_orphan_app_and_multiple_app_groups() que valida ambos os erros simultaneamente.

Copilot · 2026-01-22T11:42:10Z

packtools/sps/validation/app_group.py

+        """
+        Valida presença obrigatória do elemento <label> em <app>.
+
+        Regra SciELO: "<app> exigem o elemento <label> como título do apêndice ou anexo"


Grammar error in Portuguese docstring. The verb "exigem" (plural) should be "exige" (singular) to agree with the singular subject "". The correct sentence should be: " exige o elemento como título do apêndice ou anexo"

Suggested change

Regra SciELO: "<app> exigem o elemento <label> como título do apêndice ou anexo"

Regra SciELO: "<app> exige o elemento <label> como título do apêndice ou anexo"

Corrigido. Ajustada concordância verbal na docstring do método validate_app_label().

robertatakenaka · 2026-01-26T13:38:32Z

packtools/sps/validation/app_group.py

+                    advice_text=None,
+                    advice_params=None


@Rossi-Luciano advice nunca deveria ser None, sempre tem que ter alguma instrução para o usuário mesmo que seja óbvia, no entanto, se for "óbvia" pode ser melhorada, por exemplo, no lugar de dizer complete com o autor, explique use a tag X para representar o nome do autor e Y para representar o sobrenome, use ... para ....

@robertatakenaka esse caso se refere ao "sucesso" da validação, ou seja, existe <app> e serão geradas respostas identificando cada ocorrência da tag.

robertatakenaka · 2026-01-26T13:39:47Z

packtools/sps/validation/app_group.py

+                advice=f'Add <label> element to <app id="{app.get("id", "?")}">. '
+                       f'Example: <app id="{app.get("id", "app1")}"><label>Appendix 1</label></app>',
+                data=app,


@Rossi-Luciano user variáveis no lugar de app.get... para evitar problemas com aspas incompatíveis. Além disso esta prática deixa o código mais enxuto e evita ter app.get repetidas vezes

Implementado. Método validate_app_label() usa variáveis app_id_or_placeholder e app_id_or_example_fix para evitar repetição.

robertatakenaka · 2026-01-26T13:43:02Z

packtools/sps/validation/app_group.py

+        # Buscar <app> que estão diretamente em <back>, sem <app-group>
+        orphan_apps = self.xmltree.xpath(".//back/app")
+
+        if orphan_apps:


@Rossi-Luciano acho que uma coisa não anula a outra. Acho que na pior das hipóteses, pode haver órfãs e app dentro de app-group. Que tal dividir em 2 validações separadas e fazer uma chamas às 2 funções?

Refatorado. validate_app_group_wrapper() agora delega para validate_orphan_apps() e validate_multiple_app_groups(), garantindo separação de responsabilidades.

robertatakenaka · 2026-01-26T13:44:31Z

packtools/sps/validation/app_group.py

+        for app in self.apps:
+            media_list = app.get("media", [])
+
+            for media in media_list:


@Rossi-Luciano será que não é possível reuso de validação? (no caso MediaValidation ?)

Implementado. Removido validate_media_accessibility() pois validações de acessibilidade já são tratadas por AccessibilityDataValidation (via VisualResourceBaseValidation). Removidos também os parâmetros media_alt_text_error_level e media_transcript_error_level de app_group_rules.json.

…e media

…o de media

robertatakenaka · 2026-02-02T11:56:44Z

packtools/sps/validation/app_group.py

+                advice="Consider adding an <app> element to include additional content such as appendices.",
                data=None,
                error_level=self.params["app_existence_error_level"],
+                advice_text="Consider adding an <app> element to include additional content such as appendices.",


@Rossi-Luciano adicionar a marca de tradução

robertatakenaka · 2026-02-02T11:57:08Z

packtools/sps/validation/app_group.py

+                advice='Add @id attribute to <app>. Example: <app id="app1">',
+                data=app,
+                error_level=self.params["app_id_error_level"],
+                advice_text='Add @id attribute to <app>. Example: <app id="app1">',


@Rossi-Luciano marca de tradução

Rossi-Luciano added 3 commits January 20, 2026 17:14

Rossi-Luciano requested a review from robertatakenaka January 20, 2026 20:21

robertatakenaka requested a review from Copilot January 22, 2026 11:36

Copilot started reviewing on behalf of robertatakenaka January 22, 2026 11:36 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

robertatakenaka reviewed Jan 26, 2026

View reviewed changes

Rossi-Luciano added 3 commits February 1, 2026 12:05

refactor: separa validações app-group e remove checagens duplicadas d…

a2e0005

…e media

chore: remove parâmetros obsoletos de validação de media

0aee814

test: adiciona teste de cenário combinado e remove testes de validaçã…

a54b33e

…o de media

Rossi-Luciano requested a review from robertatakenaka February 1, 2026 15:12

robertatakenaka reviewed Feb 2, 2026

View reviewed changes

	Regra SciELO: "<app> exigem o elemento <label> como título do apêndice ou anexo"
	Regra SciELO: "<app> exige o elemento <label> como título do apêndice ou anexo"

Conversation

Rossi-Luciano commented Jan 20, 2026

O que esse PR faz?

Onde a revisão poderia começar?

Como este poderia ser testado manualmente?

Algum cenário de contexto que queira dar?

Screenshots

Quais são tickets relevantes?

Referências

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertatakenaka Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robertatakenaka Jan 26, 2026 •

edited

Loading