Skip to content

Adiciona validação de recomendação MathML e corrige validação de codificação em fórmulas#1081

Merged
robertatakenaka merged 8 commits intomasterfrom
copilot/add-validations-for-formulas
Feb 20, 2026
Merged

Adiciona validação de recomendação MathML e corrige validação de codificação em fórmulas#1081
robertatakenaka merged 8 commits intomasterfrom
copilot/add-validations-for-formulas

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 12, 2026

O que esse PR faz?

Implementa validate_mathml_recommendation para <disp-formula> e <inline-formula>, completando a conformidade com SPS 1.10. Das 6 regras listadas no issue, 5 já estavam implementadas. Este PR adiciona a última: emitir WARNING quando fórmulas contêm apenas <tex-math> sem <mml:math>, recomendando MathML para acessibilidade com leitores de tela.

Adicionalmente, corrige a lógica de validação em validate_codification para aceitar corretamente fórmulas com múltiplas codificações quando devidamente envolvidas em <alternatives>, conforme especificação SPS 1.10.

Comportamento validate_mathml_recommendation:

  • Retorna resposta OK se não há codificação (mensagem reflete "no codification found")
  • Retorna resposta OK se mml:math existe (válido)
  • Retorna WARNING se apenas tex-math existe

Comportamento validate_codification (corrigido):

  • Aceita codificação única (sem necessidade de alternatives)
  • Aceita múltiplas codificações quando todas estão envolvidas em <alternatives>
  • Rejeita múltiplas codificações sem alternatives

Atualizações importantes:

  • A validação agora sempre retorna um dicionário build_response em todos os cenários, seguindo o padrão estabelecido do codebase, em vez de retornar None em alguns casos.
  • Aplicados parênteses extras na lógica de validação de inline-formula para melhor legibilidade do código (linha 607).
# Exemplo de XML que gera WARNING (mathml_recommendation)
<disp-formula id="e1">
    <label>(1)</label>
    <tex-math>\[ E = mc^2 \]</tex-math>
</disp-formula>

# Válido - codificação única
<disp-formula id="e1">
    <label>(1)</label>
    <mml:math id="m1">...</mml:math>
</disp-formula>

# Válido - múltiplas codificações em alternatives
<disp-formula id="e1">
    <label>(1)</label>
    <alternatives>
        <mml:math id="m1">...</mml:math>
        <tex-math>...</tex-math>
    </alternatives>
</disp-formula>

# Inválido - múltiplas codificações sem alternatives
<disp-formula id="e1">
    <label>(1)</label>
    <mml:math id="m1">...</mml:math>
    <tex-math>...</tex-math>
</disp-formula>

Onde a revisão poderia começar?

packtools/sps/validation/formula.py:

  • Linhas 295-361 e 687-752 (método validate_mathml_recommendation em ambas as classes de validação)
  • Linhas 215 e 607 (lógica validate_codification corrigida com formatação aprimorada)

Como este poderia ser testado manualmente?

python -m unittest tests.sps.validation.test_formula.ArticleFormulaValidationTest -v

Todos os 21 testes devem passar (15 existentes + 6 atualizados).

Verificação manual adicional:

# Testar múltiplas codificações em alternatives (deve ser válido)
<disp-formula id="e1">
    <alternatives>
        <mml:math id="m1">x</mml:math>
        <tex-math>x</tex-math>
    </alternatives>
</disp-formula>

Algum cenário de contexto que queira dar?

A duplicação de código entre DispFormulaValidation e InlineFormulaValidation é intencional e segue o padrão existente do codebase onde ambas as classes duplicam implementações de todos os métodos de validação.

Após revisão de código, a validação foi atualizada para sempre retornar um dicionário build_response explícito em todos os cenários (OK, WARNING ou erro), em vez de retornar None em casos válidos. Isso torna o comportamento mais transparente e consistente com o padrão de validação do projeto.

A correção em validate_codification permite que fórmulas com múltiplas codificações (por exemplo, MathML e TeX para compatibilidade) sejam validadas corretamente quando envolvidas em <alternatives>, conforme especificado no SPS 1.10 para acessibilidade e compatibilidade. A formatação da expressão lógica foi aprimorada com parênteses extras para melhor legibilidade.

Referências

Original prompt

This section details on the original issue you should resolve

<issue_title>Complementar as validações para os elementos e </issue_title>
<issue_description>## Objetivo

Implementar validações faltantes para <disp-formula> e <inline-formula> conforme SPS 1.10, elevando a conformidade de 50% para 100% (6 de 12 regras implementadas atualmente).


Conformidade atual

Regra Elemento Nivel Status
validate_id disp-formula CRITICAL Implementada
validate_label disp-formula WARNING Implementada
validate_codification disp-formula CRITICAL Implementada
validate_alternatives disp-formula CRITICAL Implementada
validate_codification inline-formula CRITICAL Implementada
validate_alternatives inline-formula CRITICAL Implementada
validate_id inline-formula CRITICAL Ausente
validate_mml_math_id ambos ERROR Ausente
validate_id_prefix ambos WARNING Ausente
validate_mml_math_id_prefix ambos WARNING Ausente
validate_mathml_recommendation ambos WARNING Ausente
posicionamento apos primeira chamada disp-formula WARNING Fora do escopo

Regras a implementar

P0 - Criticas

Regra 7: validate_id em InlineFormulaValidation

InlineFormulaValidation.validate() nao chama validate_id. O metodo precisa ser adicionado seguindo o mesmo padrao ja implementado em DispFormulaValidation.validate_id(). O campo id ja esta disponivel em self.data.

Regra 8: validate_mml_math_id em ambas as classes

<mml:math> deve ter @id obrigatorio (SPS 1.10). O model Formula nao expoe esse atributo. E necessario:

  1. Adicionar a propriedade mml_math_id em packtools/sps/models/formula.py:
@property
def mml_math_id(self):
    namespace = "{http://www.w3.org/1998/Math/MathML}"
    el = self.element.find(f".//{namespace}math")
    return el.get("id") if el is not None else None
  1. Incluir "mml_math_id": self.mml_math_id no dict retornado por Formula.data.

  2. Implementar validate_mml_math_id() em DispFormulaValidation e InlineFormulaValidation, retornando None quando mml_math for None.


P1 - Importantes

Regra 9: validate_id_prefix

O @id de <disp-formula> e <inline-formula> deve iniciar com o prefixo "e". Validar com str.startswith("e"). Nivel: WARNING.

Regra 10: validate_mml_math_id_prefix

O @id de <mml:math> deve iniciar com o prefixo "m". Depende da regra 8. Retornar None quando mml_math_id for None. Nivel: WARNING.

Regra 11: validate_mathml_recommendation

Quando a formula contem tex_math e nao contem mml_math, emitir aviso recomendando MathML por acessibilidade. Retornar None quando nao houver nenhuma codificacao. Nivel: WARNING.


P2 - Fora do escopo

Regra Motivo
Posicionamento de <disp-formula> apos a primeira chamada no texto Requer rastreamento de <xref> por ordem de aparecimento e logica condicional para excluir <app-group> e <supplementary-material>. Alta complexidade, baixo volume de erros.

Arquivos

Acao Arquivo
Modificar packtools/sps/models/formula.py
Modificar packtools/sps/validation/formula.py
Modificar tests/sps/validation/test_formula.py
Criar/atualizar packtools/sps/validation/rules/formula_rules.json

Novos parametros de rules

Adicionar ao formula_rules.json e ao get_default_params de cada classe como fallback:

id_error_level                 -> "CRITICAL"  (inline-formula)
mml_math_id_error_level        -> "ERROR"      (ambos)
id_prefix_error_level          -> "WARNING"    (ambos)
mml_math_id_prefix_error_level -> "WARNING"    (ambos)
mathml_error_level             -> "WARNING"    (ambos)

Exemplos de XML

Valido - disp-formula completa:

<disp-formula id="e1">
  <label>(1)</label>
  <mml:math id="m1" display="block">
    <mml:mrow>
      <mml:mi>x</mml:mi><mml:mo>=</mml:mo><mml:mn>2</mml:mn>
    </mml:mrow>
  </mml:math>
</disp-formula>

Valido - inline-formula com id:

<inline-formula id="e3">
  <mml:math id="m3" display="inline">
    <mml:msup><mml:mi>x</mml:mi><mml:mn>2</mml:mn></mml:msup>
  </mml:math>
</inline-formula>

Invalido - inline-formula sem @id (regra 7, CRITICAL):

<inline-formula>
  <mml:math id="m3">
    <mml:msup><mml:mi>x</mml:mi><mml:mn>2</mml:mn></mml:msup>
  </mml:math>
</inline-formula>

Invalido - mml:math sem @id (regra 8, ERROR):

<disp-formula id="e1">
  <label>(1)</label>
  <mml:math display="block">
    <mml:mrow><mml:mi>x</mml:mi></mml:mrow>
  </mml:math>
</disp-formula>

Invalido - @id com prefixo incorreto (regras 9 e 10, WARNING):

<disp-formula id="eq1">
  <label>(1)</label>
  <mml:math id="math1">
    <mml:mrow><mml:mi>x</mml:mi></mml...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes scieloorg/packtools#1079

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/scieloorg/packtools/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Implement missing validations for disp-formula and inline-formula Adiciona validação de recomendação MathML para acessibilidade em fórmulas Feb 12, 2026
@scieloorg scieloorg deleted a comment from Rossi-Luciano Feb 19, 2026
Comment thread packtools/sps/validation/formula.py Outdated

# Return None if there's no codification at all
if not has_mml_math and not has_tex_math:
return None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot as validações devem sempre retornar um dicionário (padrão build_response), neste caso não foram encontrados mml_math e tex_math, assim a mensagem deverá refletir essa condição.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigido em b293ce6. A validação agora sempre retorna um dicionário build_response:

  • OK quando não há codificação (mensagem: "no codification found")
  • WARNING quando há apenas tex-math (recomenda MathML)
  • OK quando há mml:math (caso válido)

Todos os testes atualizados e passando (21/21).

Comment thread packtools/sps/validation/formula.py Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# substituir por: alternatives = self.data.get("alternative_elements") or [] is_valid = count == 1 or (count > 1 and len(alternatives) > 1)

Os métodos validate_codification, a linha is_valid = count == 1 deve ser substituída pela expressão acima. A variável alternative_elements já está disponível em self.data (conforme evidenciado pelos dados no relatório de erros), portanto não é necessário nenhum outro ajuste no método.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigido em 844cbcd. A validação validate_codification agora aceita:

  • count == 1 (codificação única)
  • count > 1 and count == len(alternatives) (múltiplas codificações em alternatives)

Testes confirmam comportamento correto para todos os cenários.

Comment thread packtools/sps/validation/formula.py Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# substituir por: alternatives = self.data.get("alternative_elements") or [] is_valid = count == 1 or (count > 1 and len(alternatives) > 1)

Os métodos validate_codification, a linha is_valid = count == 1 deve ser substituída pela expressão acima. A variável alternative_elements já está disponível em self.data (conforme evidenciado pelos dados no relatório de erros), portanto não é necessário nenhum outro ajuste no método.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigido em 844cbcd. A validação validate_codification agora aceita:

  • count == 1 (codificação única)
  • count > 1 and count == len(alternatives) (múltiplas codificações em alternatives)

Testes confirmam comportamento correto para todos os cenários.

Comment thread packtools/sps/validation/formula.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot substituir:

is_valid = count == 1

por

alternatives = self.data.get("alternative_elements") or []
is_valid = (count == 1) or (count == len(alternatives) and count > 1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Já implementado em 844cbcd. A lógica is_valid = (count == 1) or (count == len(alternatives) and count > 1) está aplicada na linha 215.

Comment thread packtools/sps/validation/formula.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot substituir:

is_valid = count == 1

por

alternatives = self.data.get("alternative_elements") or []
is_valid = (count == 1) or ((count == len(alternatives)) and (count > 1))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigido em 0c831bc. Adicionados parênteses extras para melhor legibilidade: is_valid = (count == 1) or ((count == len(alternatives)) and (count > 1))

Copilot AI changed the title Adiciona validação de recomendação MathML para acessibilidade em fórmulas Adiciona validação de recomendação MathML e corrige validação de codificação em fórmulas Feb 19, 2026
@Rossi-Luciano
Copy link
Copy Markdown
Collaborator

@roberta os testes estão OK

@robertatakenaka robertatakenaka marked this pull request as ready for review February 20, 2026 12:09
Copilot AI review requested due to automatic review settings February 20, 2026 12:09
@robertatakenaka robertatakenaka merged commit cd58b79 into master Feb 20, 2026
4 checks passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the validate_mathml_recommendation method to both DispFormulaValidation and InlineFormulaValidation classes to recommend MathML usage for accessibility when formulas contain only TeX math. Additionally, it attempts to fix the validate_codification logic to properly accept multiple codifications when wrapped in <alternatives>.

Changes:

  • Adds validate_mathml_recommendation method to emit WARNING when formulas have only tex-math without mml:math
  • Modifies validate_codification logic to accept multiple codifications when properly wrapped in alternatives
  • Adds mathml_error_level configuration to formula validation rules
  • Includes 6 new test cases for the mathml recommendation feature

Reviewed changes

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

File Description
packtools/sps/validation/formula.py Adds validate_mathml_recommendation methods and modifies validate_codification logic for both DispFormulaValidation and InlineFormulaValidation classes
tests/sps/validation/test_formula.py Adds 6 test cases covering MathML recommendation scenarios (with only tex, with mml, with both, and without codification)
packtools/sps/validation_rules/formula_rules.json Adds mathml_error_level configuration with WARNING severity
.gitignore Adds entry for src/scielo-scholarly-data (unrelated to PR purpose)

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


is_valid = count == 1
alternatives = self.data.get("alternative_elements") or []
is_valid = (count == 1) or ((count == len(alternatives)) and (count > 1))
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation logic for multiple codifications is incorrect. The condition count == len(alternatives) will fail when alternatives contains elements other than codifications (e.g., graphics). For example, a formula with both mml:math and tex-math inside alternatives along with a graphic would be incorrectly flagged as invalid because count=2 but len(alternatives)=3.

The correct logic should check whether multiple codifications exist within an alternatives element, not whether the count matches the total number of elements in alternatives. The condition should be: (count == 1) or (count > 1 and len(alternatives) > 0) to accept single codifications or multiple codifications when wrapped in alternatives.

Suggested change
is_valid = (count == 1) or ((count == len(alternatives)) and (count > 1))
is_valid = (count == 1) or (count > 1 and len(alternatives) > 0)

Copilot uses AI. Check for mistakes.
Comment on lines +307 to +323
# No codification found - return OK response reflecting this condition
if not has_mml_math and not has_tex_math:
return build_response(
title="MathML recommendation",
parent=self.data,
item="mml:math",
sub_item=None,
validation_type="exist",
is_valid=True,
expected="mml:math or tex-math",
obtained=_("no codification found"),
advice=None,
data=self.data,
error_level=self.rules["mathml_error_level"],
advice_text=None,
advice_params=None,
)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a formula has no codification (neither mml:math nor tex-math), validate_mathml_recommendation returns an OK response. This creates noise in validation results because:

  1. The formula without codification is already flagged by validate_codification as an error
  2. Returning OK for a non-applicable check adds an extra validation result that provides no useful information

Consider returning None instead of OK when there's no codification, following the pattern used by validate_mml_math_id (line 242) which returns None when the element being validated doesn't exist. This would keep validation results focused on actionable information.

Copilot uses AI. Check for mistakes.
Comment on lines +699 to +715
# No codification found - return OK response reflecting this condition
if not has_mml_math and not has_tex_math:
return build_response(
title="MathML recommendation",
parent=self.data,
item="mml:math",
sub_item=None,
validation_type="exist",
is_valid=True,
expected="mml:math or tex-math",
obtained=_("no codification found"),
advice=None,
data=self.data,
error_level=self.rules["mathml_error_level"],
advice_text=None,
advice_params=None,
)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a formula has no codification (neither mml:math nor tex-math), validate_mathml_recommendation returns an OK response. This creates noise in validation results because:

  1. The formula without codification is already flagged by validate_codification as an error
  2. Returning OK for a non-applicable check adds an extra validation result that provides no useful information

Consider returning None instead of OK when there's no codification, following the pattern used by validate_mml_math_id (line 624) which returns None when the element being validated doesn't exist. This would keep validation results focused on actionable information.

Copilot uses AI. Check for mistakes.
mathml_responses = [item for item in obtained if item["title"] == "MathML recommendation"]
self.assertEqual(len(mathml_responses), 1)
self.assertEqual(mathml_responses[0]["response"], "OK")
self.assertIn("no codification found", mathml_responses[0]["got_value"])
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage is missing for the scenario where multiple codifications (mml:math and tex-math) are wrapped in alternatives along with other elements like graphics. This is a valid use case according to SPS 1.10 documentation but would be incorrectly flagged as invalid by the current validate_codification logic.

Add a test case like:

<disp-formula id="e1">
    <alternatives>
        <mml:math id="m1">...</mml:math>
        <tex-math id="tx1">...</tex-math>
        <graphic xlink:href="image.png"/>
    </alternatives>
</disp-formula>

This should validate successfully but the current logic would fail it.

Suggested change
self.assertIn("no codification found", mathml_responses[0]["got_value"])
self.assertIn("no codification found", mathml_responses[0]["got_value"])
def test_validate_disp_formula_alternatives_with_multiple_codifications_and_graphic(self):
"""Test that disp-formula with alternatives containing mml:math, tex-math and graphic is accepted"""
self.maxDiff = None
xml_tree = etree.fromstring(
'<article xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:mml="http://www.w3.org/1998/Math/MathML" '
'dtd-version="1.0" article-type="research-article" xml:lang="pt">'
"<body>"
'<disp-formula id="e1">'
"<alternatives>"
'<mml:math id="m1">x^2</mml:math>'
'<tex-math id="tx1">x^2</tex-math>'
'<graphic xlink:href="https://github.com/image.png"/>'
"</alternatives>"
"</disp-formula>"
"</body>"
"</article>"
)
obtained = list(
ArticleDispFormulaValidation(
xml_tree=xml_tree, rules={"mathml_error_level": "WARNING"}
).validate()
)
# Deve aceitar a combinação de múltiplas codificações e gráfico em alternatives
errors = [item for item in obtained if item["response"] == "ERROR"]
self.assertEqual(errors, [])

Copilot uses AI. Check for mistakes.

is_valid = count == 1
alternatives = self.data.get("alternative_elements") or []
is_valid = (count == 1) or (count == len(alternatives) and count > 1)
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation logic for multiple codifications is incorrect. The condition count == len(alternatives) will fail when alternatives contains elements other than codifications (e.g., graphics). For example, a formula with both mml:math and tex-math inside alternatives along with a graphic would be incorrectly flagged as invalid because count=2 but len(alternatives)=3.

The correct logic should check whether multiple codifications exist within an alternatives element, not whether the count matches the total number of elements in alternatives. The condition should be: (count == 1) or (count > 1 and len(alternatives) > 0) to accept single codifications or multiple codifications when wrapped in alternatives.

Suggested change
is_valid = (count == 1) or (count == len(alternatives) and count > 1)
is_valid = (count == 1) or (count > 1 and len(alternatives) > 0)

Copilot uses AI. Check for mistakes.
Rossi-Luciano pushed a commit to Rossi-Luciano/packtools that referenced this pull request Mar 19, 2026
…ficação em fórmulas (scieloorg#1081)

* Initial plan

* Initial plan for adding validate_mathml_recommendation

Co-authored-by: robertatakenaka <[email protected]>

* Add validate_mathml_recommendation to formula validations

Co-authored-by: robertatakenaka <[email protected]>

* Remove accidentally added submodule and update gitignore

* Fix validate_mathml_recommendation to always return build_response dict

Co-authored-by: robertatakenaka <[email protected]>

* Remove unnecessary ternary operator in obtained field

Co-authored-by: robertatakenaka <[email protected]>

* Fix validate_codification to accept multiple codifications in alternatives

Co-authored-by: robertatakenaka <[email protected]>

* Add extra parentheses for clarity in inline-formula codification validation

Co-authored-by: robertatakenaka <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: robertatakenaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants