Skip to content

Conversation

@Rossi-Luciano
Copy link
Collaborator

Este PR implementa as validações obrigatórias da especificação SPS 1.10 para os elementos <disp-formula> e <inline-formula>, incluindo validações de prefixos de IDs e atributos obrigatórios em elementos internos como <mml:math>.

Principais adições:

  • 6 novas validações para conformidade com regras obrigatórias SPS
  • Migração de format_response para build_response para suporte completo a internacionalização (i18n)
  • Adição de propriedades mml_math_id e tex_math_id ao modelo
  • 7 novos testes unitários para cobertura das novas validações
  • Configuração completa de níveis de erro (CRITICAL/ERROR/WARNING)

Cobertura: 87,5% das regras obrigatórias SPS (7/8 implementadas)

Onde a revisão poderia começar?

Ordem sugerida de revisão:

  1. Modelo: packtools/sps/models/formula.py - Novas propriedades mml_math_id e tex_math_id
  2. Validação: packtools/sps/validation/formula.py - 10 validações (linhas 121-700)
  3. Configuração: packtools/sps/validation/formula_rules.json - Níveis de erro
  4. Testes: tests/sps/validation/test_formula.py - 15 testes (7 novos)

Pontos de atenção:

  • Mudança de format_response para build_response (breaking change)
  • Uso de parent=self.data em vez de parâmetros individuais

Como este poderia ser testado manualmente?

1. Executar suite de testes:

# Testes de validação (15 testes)
python -m unittest tests.sps.validation.test_formula -v

# Testes do modelo (múltiplos testes)
python -m unittest tests.sps.models.test_formula -v

# Resultado esperado: todos os testes passam

2. Testar validação de prefixo incorreto:

from lxml import etree
from packtools.sps.validation.formula import ArticleDispFormulaValidation

xml = '''<article xmlns:mml="http://www.w3.org/1998/Math/MathML">
    <body>
        <disp-formula id="f10">  <!-- Prefixo errado -->
            <mml:math id="e03">...</mml:math>  <!-- Prefixo errado -->
        </disp-formula>
    </body>
</article>'''

xml_tree = etree.fromstring(xml)
validator = ArticleDispFormulaValidation(xml_tree, {})

for error in validator.validate():
    if error["response"] != "OK":
        print(f"{error['response']}: {error['advice']}")

# Deve retornar 2 erros de prefixo

3. Testar ID ausente em mml:math:

xml = '''<article xmlns:mml="http://www.w3.org/1998/Math/MathML">
    <body>
        <disp-formula id="e10">
            <mml:math>...</mml:math>  <!-- Sem @id -->
        </disp-formula>
    </body>
</article>'''

# Deve retornar erro CRITICAL

Algum cenário de contexto que queira dar?

Contexto SPS:
A especificação SciELO Publishing Schema (SPS) 1.10 define regras obrigatórias para identificação de fórmulas matemáticas em artigos científicos. Essas regras garantem:

  • Rastreabilidade única de cada fórmula via @id
  • Padronização de nomenclatura com prefixos específicos ("e" para fórmulas, "m" para mml:math)
  • Validação de estrutura correta de elementos alternativos

Motivação técnica:
A migração de format_response para build_response foi necessária porque:

  • format_response ignora silenciosamente os parâmetros advice_text e advice_params
  • Sistema de i18n requer templates não formatados para tradução
  • Permite extração automática de strings para arquivos .po/.pot

Breaking Change:
O campo retornado mudou de obtained para got_value. Testes que verificam valores devem usar error["got_value"] em vez de error["obtained"].

Screenshots

Não aplicável - validações de XML retornam estruturas de dados.

Quais são tickets relevantes?

N.A.

Referências

N.A.

Copy link
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

Implementa validações obrigatórias da SPS 1.10 para <disp-formula> e <inline-formula>, adicionando checagens de prefixos de IDs e atributos obrigatórios (ex.: @id em <mml:math>), além de ajustar o formato de resposta para suportar i18n via build_response.

Changes:

  • Adiciona validações de prefixo/obrigatoriedade de @id para fórmulas e para <mml:math> (disp e inline).
  • Estende o modelo de fórmula com mml_math_id e tex_math_id.
  • Atualiza regras (níveis CRITICAL/ERROR/WARNING) e amplia/ajusta testes unitários para as novas validações e campos i18n.

Reviewed changes

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

Show a summary per file
File Description
packtools/sps/validation/formula.py Inclui novas validações SPS 1.10 (prefixos e @id em mml:math) e migra respostas para build_response com suporte a i18n.
packtools/sps/models/formula.py Adiciona propriedades mml_math_id e tex_math_id ao modelo e expõe no data.
packtools/sps/validation_rules/formula_rules.json Configura novos níveis de severidade para as validações adicionadas.
tests/sps/validation/test_formula.py Atualiza testes para novo payload (msg/adv templates) e adiciona cenários para prefixos e @id em mml:math.
tests/sps/models/test_formula.py Ajusta expectativas dos testes do modelo para incluir os novos campos mml_math_id/tex_math_id.

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

Comment on lines 164 to 166
for i, item in enumerate(expected):
with self.subTest(i):
self.assertDictEqual(item, obtained[i])
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

This test only asserts that obtained[0] matches the expected "@id" error and doesn’t verify that no additional non-OK validations were returned (new prefix validations can add extra ERROR items). Consider filtering obtained by title == "@id" (like the newer tests) and/or asserting there are no unexpected non-OK responses so the test fails if new mandatory validations start triggering.

Copilot uses AI. Check for mistakes.
Comment on lines +159 to +162
advice=_('The @id of <disp-formula> must start with prefix "e". Change {id} to e{id} in <disp-formula id="{id}">. Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <disp-formula> must start with prefix "e". Change {id} to e{id} in <disp-formula id="{id}">. Consult SPS documentation for more detail.'),
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The prefix-fix suggestion in the advice will produce values like "ef10" for an ID "f10" (it prepends "e" rather than making the ID start with "e"). Update the advice template/params to suggest replacing the leading character or otherwise generating an example that results in a valid SPS-style ID (e.g., "e10").

Suggested change
advice=_('The @id of <disp-formula> must start with prefix "e". Change {id} to e{id} in <disp-formula id="{id}">. Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <disp-formula> must start with prefix "e". Change {id} to e{id} in <disp-formula id="{id}">. Consult SPS documentation for more detail.'),
advice=_('The @id of <disp-formula> must start with prefix "e". Update the value of id in <disp-formula id="{id}"> so that it starts with "e" (for example, change "f10" to "e10"). Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <disp-formula> must start with prefix "e". Update the value of id in <disp-formula id="{id}"> so that it starts with "e" (for example, change "f10" to "e10"). Consult SPS documentation for more detail.'),

Copilot uses AI. Check for mistakes.
Comment on lines +286 to +289
advice=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.').format(mml_id=mml_math_id, formula_id=item_id),
data=self.data,
error_level=self.rules["mml_math_id_prefix_error_level"],
advice_text=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.'),
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The advice suggests changing an ID like "e03" to "me03" (by prepending "m"), which still won’t match the expected "m"-prefixed pattern used elsewhere (e.g., "m03"). Adjust the advice template so the suggested change results in an ID that actually starts with "m" (replace the prefix rather than prepend).

Suggested change
advice=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.').format(mml_id=mml_math_id, formula_id=item_id),
data=self.data,
error_level=self.rules["mml_math_id_prefix_error_level"],
advice_text=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.'),
advice=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} so that it starts with "m" (for example, "m03" instead of "e03") in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.').format(mml_id=mml_math_id, formula_id=item_id),
data=self.data,
error_level=self.rules["mml_math_id_prefix_error_level"],
advice_text=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} so that it starts with "m" (for example, "m03" instead of "e03") in <mml:math id="{mml_id}"> inside <disp-formula id="{formula_id}">. Consult SPS documentation for more detail.'),

Copilot uses AI. Check for mistakes.
dict or None: A validation result dictionary if the validation fails; otherwise, None.
"""
# Só valida se houver mml:math
if not self.data.get("mml_math"):
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

validate_mml_math_id (and similarly validate_mml_math_id_prefix) uses if not self.data.get("mml_math") to decide whether a mml:math exists. If the element exists but its text serialization is an empty string, this check incorrectly skips required @id validations. Use an explicit is None check (since the model returns None when the element is absent) rather than relying on truthiness.

Suggested change
if not self.data.get("mml_math"):
if self.data.get("mml_math") is None:

Copilot uses AI. Check for mistakes.
Comment on lines +558 to +561
# Só valida se houver mml:math
if not self.data.get("mml_math"):
return None

Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Same issue as in disp-formula: using if not self.data.get("mml_math") treats an empty-string MathML serialization as “no mml:math”, causing mml:math @id validations to be skipped. Prefer checking self.data.get("mml_math") is None to distinguish “absent element” from “present but empty content”.

Copilot uses AI. Check for mistakes.
Comment on lines +507 to +510
advice=_('The @id of <inline-formula> must start with prefix "e". Change {id} to e{id} in <inline-formula id="{id}">. Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <inline-formula> must start with prefix "e". Change {id} to e{id} in <inline-formula id="{id}">. Consult SPS documentation for more detail.'),
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The prefix-fix suggestion in the advice will produce IDs like "ef10" for an existing "f10" (it prepends "e"). Update the advice template so the suggested change results in a valid "e"-prefixed ID (e.g., replace the leading character, or provide an example like "e10").

Suggested change
advice=_('The @id of <inline-formula> must start with prefix "e". Change {id} to e{id} in <inline-formula id="{id}">. Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <inline-formula> must start with prefix "e". Change {id} to e{id} in <inline-formula id="{id}">. Consult SPS documentation for more detail.'),
advice=_('The @id of <inline-formula> must start with prefix "e". Change {id} to an ID starting with "e" (for example, "e10") in <inline-formula id="{id}">. Consult SPS documentation for more detail.').format(id=formula_id),
data=self.data,
error_level=self.rules["id_prefix_error_level"],
advice_text=_('The @id of <inline-formula> must start with prefix "e". Change {id} to an ID starting with "e" (for example, "e10") in <inline-formula id="{id}">. Consult SPS documentation for more detail.'),

Copilot uses AI. Check for mistakes.
Comment on lines +607 to +611
advice=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <inline-formula id="{formula_id}">. Consult SPS documentation for more detail.').format(mml_id=mml_math_id, formula_id=item_id),
data=self.data,
error_level=self.rules["mml_math_id_prefix_error_level"],
advice_text=_('The @id of <mml:math> must start with prefix "m". Change {mml_id} to m{mml_id} in <mml:math id="{mml_id}"> inside <inline-formula id="{formula_id}">. Consult SPS documentation for more detail.'),
advice_params={"mml_id": mml_math_id, "formula_id": item_id},
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The advice currently suggests changing an ID like "e03" to "me03" by prepending "m", which is unlikely to be the intended SPS pattern (commonly "m03"). Adjust the advice template/params so the recommended change yields an ID that actually starts with "m" (replace prefix rather than prepend).

Copilot uses AI. Check for mistakes.
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.

1 participant