Add <sec> element validation (SPS 1.10, 7/10 rules)#1137
Add <sec> element validation (SPS 1.10, 7/10 rules)#1137
<sec> element validation (SPS 1.10, 7/10 rules)#1137Conversation
Implements validation for <sec> elements: - Rule 1: <title> presence (CRITICAL) - Rule 2: @sec-type valid values (ERROR) - Rule 3: @id for transcript sections (ERROR) - Rule 4: data-availability section for indexable articles (ERROR) - Rule 5: Combined sec-type pipe format (WARNING) - Rule 6: Non-combinable sec-types (WARNING) - Rule 7: Content presence (WARNING) New files: - packtools/sps/models/sec.py - Model for <sec> data extraction - packtools/sps/validation/sec.py - Validation logic - packtools/sps/validation_rules/sec_rules.json - Rules config - tests/sps/validation/test_sec.py - 29 unit tests Modified files: - packtools/sps/validation/xml_validations.py - Added validate_secs() - packtools/sps/validation/xml_validator.py - Added sec group Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
<sec> element validation (SPS 1.10, 7/10 rules)
Rossi-Luciano
left a comment
Rossi-Luciano
left a comment
There was a problem hiding this comment.
Evidência de revisão — sec.py
Data: 2026-04-24
Revisor: Luciano
Artefatos analisados:
sec.py(validation) — módulo de validação (SecValidation,XMLSecValidation)sec.py(model) —ArticleSecs,Secsec_rules.json— parâmetros de configuraçãosec.xml— XML artificial gerado para cobertura de testessec-2026-24-04-120514-errors.csv— relatório de erros gerado pelo packtools
Metodologia
Análise cruzada entre os dois módulos de mesmo nome (sec.py de validation e de models), o JSON de regras, o XML artificial construído com o princípio do defeito isolado, e o relatório CSV. O model foi necessário para confirmar três pontos: (1) paragraph_count usa element.findall("p") — apenas <p> filhos diretos, não descendentes; (2) all_secs usa .//sec — inclui subsecções aninhadas, cada uma validada individualmente; (3) body_sec_types usa body.findall("sec") — apenas filhos diretos de <body>, uma <sec sec-type="data-availability"> aninhada não satisfaz a regra R4.
Para evitar ruído de R7 causado por subsecções sem <p> direto, todas as seções de teste foram posicionadas como irmãs de primeiro nível em <body>, sem subsecções.
Regras validadas (7 em SecValidation + 1 em XMLSecValidation)
| # | Classe | Método | Nível configurado |
|---|---|---|---|
| R1 | SecValidation | validate_title |
CRITICAL |
| R2 | SecValidation | validate_sec_type_value |
ERROR |
| R3 | SecValidation | validate_transcript_id |
ERROR |
| R4 | XMLSecValidation | validate_data_availability_presence |
ERROR |
| R5 | SecValidation | validate_combined_format |
WARNING |
| R6 | SecValidation | validate_non_combinable |
WARNING |
| R7 | SecValidation | validate_content |
WARNING |
Casos de teste no XML artificial
| Caso | Defeito introduzido | Nível esperado | Isolamento |
|---|---|---|---|
| P1 | Nenhum — caso ouro, sec-type="intro" com <title> e <p> |
OK (ausente do CSV) | todos os métodos passam |
| C-R1 | <sec> sem <title> |
CRITICAL | sec_type=None — R2, R3, R5, R6 retornam None (skip); R7 passa (tem <p>) |
| C-R2 | sec-type="invalid-section" (valor inválido) |
ERROR | sem espaço/vírgula/pipe — R5 e R6 não disparam |
| C-R3 | sec-type="transcript" sem @id |
ERROR | "transcript" é válido — R2 passa; sem pipe — R6 não dispara |
| C-R4 | artigo research-article sem <sec sec-type="data-availability"> como filho direto de <body> |
ERROR | regra de nível artigo, gerada por XMLSecValidation; não associada a nenhuma <sec> individual |
| C-R5 | sec-type="materials methods" (espaço sem pipe) |
R2 ERROR + R5 WARNING | cascata estrutural: espaço sem pipe faz "materials methods" não constar em valid_sec_types (R2) e ativa has_space_separator=True (R5) simultaneamente |
| C-R6 | sec-type="transcript|intro" (tipo não-combinável com pipe) |
WARNING | "transcript|intro" != "transcript" — R3 não dispara; ambos os parts são válidos — R2 passa; R5 não dispara (tem pipe) |
| C-R7 | sec-type="discussion" sem <p> direto |
WARNING | paragraph_count = len(element.findall("p")) = 0; R1 passa (tem <title>); R2 passa |
Verificação cruzada com o CSV
Todas as 8 entradas esperadas do módulo sec.py (validation) aparecem no CSV
sob o subject sec, com os níveis de severidade corretos:
| Subject | Caso | Nível | Verificação |
|---|---|---|---|
| sec | C-R1 | CRITICAL | PASS — "Add <title> element to <sec> for accessibility" |
| sec | C-R2 | ERROR | PASS — "Replace @sec-type=\"invalid-section\" with a valid value" |
| sec | C-R3 | ERROR | PASS — "Add @id attribute to <sec sec-type=\"transcript\">" |
| sec | C-R4 | ERROR | PASS — "Add <sec sec-type=\"data-availability\"...> to <body> (required for article-type=\"research-article\")" |
| sec | C-R5 cascata | ERROR | PASS — R2 dispara para "materials methods" inválido |
| sec | C-R5 | WARNING | PASS — "Use pipe | as separator in @sec-type=\"materials methods\"" |
| sec | C-R6 | WARNING | PASS — "Do not combine \"transcript\" with other types in @sec-type=\"transcript|intro\"" |
| sec | C-R7 | WARNING | PASS — "Add at least one <p> element to <sec>" |
O caso ouro P1 não gera nenhuma entrada. Não foram identificados falsos positivos nem falsos negativos originados do módulo.
Ruído identificado (outros módulos) — 20 entradas
Ruído estrutural (XML artificial mínimo): abstract ausente, history dates ausentes (received, rev-request, rev-recd, accepted, pub), subj-group heading ausente, disp-formula, inline-formula, table-wrap, fig e app ausentes.
Ruído de ambiente: rendition PDF ausente, DOI não registrado no Crossref, issue não verificada no Core.
Ruído com sobreposição identificada: o módulo open science (subject open science, CRITICAL) disparou para a mesma condição de C-R4. Isso ocorre porque dois módulos distintos verificam a declaração de disponibilidade de dados: sec.py verifica a presença estrutural de <sec sec-type="data-availability"> em <body> (subject sec), enquanto o módulo de open science verifica a declaração como um todo (subject open science). Ambas as entradas são esperadas e corretas; distinguem-se pelo campo subject no CSV.
Conclusão
O módulo sec.py (validation) está correto. Todas as 8 entradas esperadas
dispararam com os níveis de severidade corretos, sem falsos positivos nem falsos negativos originados do módulo. A cascata C-R5 (R2 ERROR + R5 WARNING para sec-type="materials methods") é estrutural e esperada: o formato incorreto invalida o valor simultaneamente para as duas regras.
There was a problem hiding this comment.
Pull request overview
Implements SPS 1.10 validations for the <sec> element by introducing a dedicated model + validator, wiring it into the XML validation pipeline, and adding a rule config file and tests.
Changes:
- Added
Sec/ArticleSecsmodel to extract<sec>data across article + translations. - Added
SecValidation/XMLSecValidationto validate per-<sec>rules and document-level data-availability presence. - Integrated a new
secgroup into the validation pipeline and added comprehensive unit tests + JSON rules config.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packtools/sps/models/sec.py |
New model to collect <sec> data for validation, including traversal across translations. |
packtools/sps/validation/sec.py |
New validation logic for <sec> rules + doc-level data-availability presence check. |
packtools/sps/validation_rules/sec_rules.json |
Default rule parameters (error levels, allowed sec-types, etc.). |
packtools/sps/validation/xml_validations.py |
Adds validate_secs() entrypoint that runs XMLSecValidation. |
packtools/sps/validation/xml_validator.py |
Adds the sec group to the validation output pipeline. |
tests/sps/validation/test_sec.py |
New test suite covering the new model + validations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| VALID_SEC_TYPES = [ | ||
| "cases", | ||
| "conclusions", | ||
| "data-availability", | ||
| "discussion", | ||
| "intro", | ||
| "materials", | ||
| "methods", | ||
| "results", | ||
| "subjects", | ||
| "supplementary-material", | ||
| "transcript", | ||
| ] | ||
|
|
||
| NON_COMBINABLE_SEC_TYPES = [ | ||
| "data-availability", | ||
| "supplementary-material", | ||
| "transcript", | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
VALID_SEC_TYPES and NON_COMBINABLE_SEC_TYPES are defined but not referenced anywhere in the PR (validation relies on params from sec_rules.json). If these constants are not meant to be part of the public API, consider removing them to avoid dead code, or alternatively use them as defaults in SecValidation/sec_rules.json to keep the source of truth in one place.
| VALID_SEC_TYPES = [ | |
| "cases", | |
| "conclusions", | |
| "data-availability", | |
| "discussion", | |
| "intro", | |
| "materials", | |
| "methods", | |
| "results", | |
| "subjects", | |
| "supplementary-material", | |
| "transcript", | |
| ] | |
| NON_COMBINABLE_SEC_TYPES = [ | |
| "data-availability", | |
| "supplementary-material", | |
| "transcript", | |
| ] |
| def _get_secs_from_node(self, node, parent_attribs): | ||
| for sec_elem in node.xpath(".//sec"): | ||
| yield Sec(sec_elem, parent_attribs).data |
There was a problem hiding this comment.
ArticleSecs.all_secs() uses node.xpath('.//sec') for the main article node, which will also traverse into <sub-article> descendants. Since the loop also iterates over translation sub-articles separately, the same <sec> elements can be yielded twice (duplicating validation results). Consider changing the XPath for the main article to exclude ancestor::sub-article (or restrict the search to ./front|./body|./back|./front-stub like other models) so each <sec> is reported only once per language/article context.
| def validate_sec_type_value(self): | ||
| """Rule 2: When present, @sec-type must have a valid value.""" | ||
| sec_type = self.data.get("sec_type") | ||
| if not sec_type: | ||
| return None | ||
|
|
||
| valid_sec_types = self.params.get("valid_sec_types", []) | ||
| # Handle combined types (e.g. "materials|methods") | ||
| parts = sec_type.split("|") | ||
| is_valid = all(part in valid_sec_types for part in parts) | ||
|
|
There was a problem hiding this comment.
validate_sec_type_value() treats sec-type="materials methods" (or comma-separated) as a single invalid token and returns an ERROR, and validate_combined_format() will also emit a WARNING. This conflicts with the PR description where wrong separators are intended to be a WARNING when the underlying types are valid. A concrete fix is to normalize separators (split on |, space, and ,), strip tokens, validate token values against valid_sec_types, and let only the combined-format rule report the separator problem.
| def validate_data_availability_presence(self): | ||
| """Rule 4: Certain article types require a data-availability section.""" | ||
| required_types = self.params.get( | ||
| "data_availability_required_article_types", | ||
| [ | ||
| "data-article", | ||
| "brief-report", | ||
| "case-report", | ||
| "rapid-communication", | ||
| "research-article", | ||
| "review-article", | ||
| ], | ||
| ) | ||
|
|
||
| article_type = self.article_secs.main_article_type | ||
| if not article_type or article_type not in required_types: | ||
| return | ||
|
|
||
| body_sec_types = self.article_secs.body_sec_types | ||
| has_data_availability = "data-availability" in body_sec_types |
There was a problem hiding this comment.
validate_data_availability_presence() currently checks only first-level <body>/<sec> types (ArticleSecs.body_sec_types). Elsewhere in the codebase, data availability is considered valid when provided as either <sec sec-type="data-availability"> or <fn fn-type="data-availability"> in <body> or <back> (see DataAvailability/DataAvailabilityValidation). As written, this can incorrectly emit an ERROR for valid documents (e.g., statement in <back> or as <fn>). Consider either reusing DataAvailabilityValidation.validate_data_availability_exist() for this rule, or expanding the search to match the same locations/types to avoid contradictory results.
O que esse PR faz?
Implementa validações para o elemento
<sec>conforme SPS 1.10, cobrindo 7 de 10 regras (70%):<title>obrigatório@sec-typevalores válidos@idem transcript<sec sec-type="transcript">exige@iddata-availabilityobrigatório<p>Novos arquivos:
packtools/sps/models/sec.py—Sec,ArticleSecspacktools/sps/validation/sec.py—SecValidation,XMLSecValidationpacktools/sps/validation_rules/sec_rules.jsontests/sps/validation/test_sec.py— 29 testesModificados:
xml_validations.py—validate_secs()xml_validator.py— gruposecno pipelineOnde a revisão poderia começar?
packtools/sps/validation/sec.py— contém toda a lógica de validação. O modelo empacktools/sps/models/sec.pyé a dependência principal.Como este poderia ser testado manualmente?
Ou via pipeline completo:
Algum cenário de contexto que queira dar?
Segue os mesmos padrões de
GraphicValidation/XMLGraphicValidation: modelo extrai dados, validação individual por elemento viaSecValidation, eXMLSecValidationorquestra + adiciona validação de nível de documento (presença dedata-availability). Regras P2 (hierarquia de@sec-type,@specific-use, ordem lógica) ficam fora do escopo conforme definido na issue.Screenshots
N/A — validação backend sem componente visual.
Quais são tickets relevantes?
Criar validações para o elemento
<sec>(SPS 1.10).Referências
<sec>Original prompt
This section details on the original issue you should resolve
<issue_title>Criar validações para o elemento </issue_title>
<issue_description>## Objetivo
Implementar validações para o elemento
<sec>conforme a especificação SPS 1.10, aumentando a conformidade de X% para 70% (7 de 10 regras).Nota: Algumas validações para
<sec>podem já estar parcialmente implementadas no repositório. Este Issue visa reavaliar, complementar e garantir cobertura completa das regras SPS 1.10.Contexto
O elemento
<sec>representa seções de texto do documento. Para acessibilidade, cada seção deve obrigatoriamente conter<title>. Seções de primeiro nível que correspondem a tipos específicos (métodos, resultados, etc.) devem ter o atributo@sec-type. Alguns tipos de documentos requerem obrigatoriamente seção de disponibilidade de dados (@sec-type="data-availability"). Validações corretas garantem acessibilidade, presença de elementos obrigatórios, e conformidade com requisitos de tipos de documentos.Conformidade atual: X de 10 regras implementadas (X%)
Meta após implementação: 7 de 10 regras (70%)
Documentação SPS
Referência oficial: https://docs.google.com/document/d/1GTv4Inc2LS_AXY-ToHT3HmO66UT0VAHWJNOIqzBNSgA/edit?tab=t.0#heading=h.sec
Regras principais conforme SPS 1.10:
Ocorrência:
<sec>pode aparecer zero ou mais vezes em:<abstract>,<app>,<back>,<bio>,<body>,<boxed-text>,<sec>,<trans-abstract>Elemento obrigatório (Acessibilidade):
<title>é mandatório em<sec>para criar títulos acessíveisAtributo
@sec-type:Valores permitidos para
@sec-type:cases- Relatos/casos/estudos de casoconclusions- Conclusões/considerações finais/comentáriosdata-availability- Declaração de Disponibilidade de Dadosdiscussion- Discussões/interpretaçõesintro- Introdução/sinopsematerials- Materiaismethods- Metodologias/métodos/procedimentosresults- Resultados/descobertassubjects- Participantes/Pacientessupplementary-material- Material suplementar/material adicionaltranscript- Transcrição de vídeo ou áudioSeções combinadas:
|como separadormaterials|methodssupplementary-material,transcriptedata-availabilitynão podem ser combinados@sec-type="data-availability"obrigatório:data-article,brief-report,case-report,rapid-communication,research-article,review-article@specific-use@sec-type="transcript"exige@id:@idobrigatórioEstrutura:
<title>) seguido de um ou mais parágrafos (<p>)Regras a Implementar
P0 – Críticas (implementar obrigatoriamente)
<title><title>é mandatório em<sec>(requisito de acessibilidade)@sec-type@sec-typedeve ter valor da lista permitida@idemtranscript@sec-type="transcript"deve ter atributo@iddata-availabilityem documentos indexáveis@sec-type="data-availability"P1 – Importantes (implementar se possível)
transcript,supplementary-materialedata-availabilitynão são combinados<p>após<title>P2 – Futuras (fora do escopo deste Issue)
@sec-typesó é usado em seções de primeiro nível@specific-usequando@sec-type="data-availability"Arquivos a Criar/Modificar
Avaliar existentes (podem ter validações parciais...
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.