Skip to content

fix(styles): correct article formatting for entries without volume/number#38

Open
wenh06 wants to merge 1 commit intomasterfrom
fix/style-formatting-edge-cases
Open

fix(styles): correct article formatting for entries without volume/number#38
wenh06 wants to merge 1 commit intomasterfrom
fix/style-formatting-edge-cases

Conversation

@wenh06
Copy link
Copy Markdown
Contributor

@wenh06 wenh06 commented Mar 31, 2026

Problem

Several citation styles produced incorrect output for journal articles that have no volume/number fields (e.g. IACR TCHES papers), and for page ranges stored with an en-dash (–).

Style Bug Example
IEEE en-dash not recognised as range → p. 21–43 should be pp. 21–43
APA missing volume caused double comma → , , 21–43 should be , 21–43
GBT year/pages separated by comma; en-dash not converted to hyphen 2018, 21–432018: 21-43
Chicago no volume → double space + parenthesised date (August 2018), August 2018,

Changes

  • ieee.pyieee_pages: also treat U+2013 (en-dash) as a range separator.
  • apa.py – use field() (raises FieldIsMissing) instead of optional_field() inside optional[] blocks for volume and pages.
  • gbt7714.py – replace inline year/vol/pages join with a new gbt_year_vol_pages node that handles all combinations and normalises page-range separators to -.
  • chicago.py – extract month map to module-level _CHICAGO_MONTH_MAP; add chicago_date_plain node; branch get_article_template on whether volume is present.

Tests

Added the TCHES 2018 DOI (10.46586/tches.v2018.i3.21-43) as a new entry in EXAMPLES covering all four styles, plus unit tests for chicago_date_plain and gbt_year_vol_pages.

- ieee: detect en-dash (–) as page range separator → pp. instead of p.
- apa: use field() inside optional[] so missing volume/pages are
  properly suppressed (avoids double comma)
- gbt7714: add gbt_year_vol_pages node to handle year/vol/pages
  formatting; normalize en-dash and double-hyphen to hyphen in
  page ranges per GB/T 7714-2015
- chicago: split get_article_template by volume presence; add
  chicago_date_plain node for comma-separated "Journal, Month Year,
  Pages" format when volume is absent

Add TCHES 2018 example (no volume, en-dash page range) to EXAMPLES
in test_styles.py; add unit tests for new nodes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 31, 2026 16:15
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@bc640a5). Learn more about missing BASE report.

Files with missing lines Patch % Lines
bib_lookup/styles/chicago.py 94.73% 1 Missing ⚠️
bib_lookup/styles/gbt7714.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master      #38   +/-   ##
=========================================
  Coverage          ?   98.47%           
=========================================
  Files             ?       13           
  Lines             ?     1841           
  Branches          ?        0           
=========================================
  Hits              ?     1813           
  Misses            ?       28           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ccee94ee1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +194 to +198
journal_info = join(sep=", ")[
tag("em")[field("journal")],
chicago_date_plain,
chicago_pages,
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve issue number when Chicago volume is absent

In ChicagoStyle.get_article_template, the no-volume branch no longer references field("number"), so entries that have an issue number but no volume now silently lose that bibliographic detail. This regression is introduced by the new if e.fields.get("volume") split: previously the template still emitted , no. N when number existed, but the new branch outputs only journal/date/pages, which can make citations ambiguous for issue-only journals.

Useful? React with 👍 / 👎.

Comment on lines +83 to +84
if volume:
vol_str = f"{volume}({number})" if number else volume
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve issue number in GBT output without volume

gbt_year_vol_pages now ignores number unless volume is present (vol_str is only built inside if volume:), so issue-only article metadata is dropped from formatted output. This is a behavior change from the previous template, which still rendered number in the article suffix, and it reduces citation accuracy for journals that provide issue but not volume.

Useful? React with 👍 / 👎.

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

Fixes formatting edge-cases in multiple citation styles for journal articles missing volume/number, and improves handling of en-dash page ranges.

Changes:

  • IEEE: treat en-dash (U+2013) as a page-range separator so pp. is used correctly.
  • APA: avoid stray punctuation when volume/pages are absent by using field() inside optional[].
  • GB/T 7714 & Chicago: refactor date/year/volume/pages formatting into dedicated nodes to handle missing fields and normalize page-range separators.

Reviewed changes

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

Show a summary per file
File Description
bib_lookup/styles/ieee.py Updates ieee_pages to recognize en-dash ranges when deciding between p./pp..
bib_lookup/styles/apa.py Switches to field() inside optional[] blocks to prevent double commas when fields are missing.
bib_lookup/styles/gbt7714.py Introduces gbt_year_vol_pages node and uses it in the article template; normalizes en-dash/-- to -.
bib_lookup/styles/chicago.py Extracts month mapping, adds chicago_date_plain, and branches article formatting when volume is absent.
test/test_styles.py Adds a TCHES example and unit tests covering new nodes and the missing-volume/page-range cases across styles.

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

Comment on lines +87 to +91
return f"{year}, {vol_str}: {pages}"
if vol_str:
return f"{year}, {vol_str}"
if pages:
return f"{year}: {pages}"
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

gbt_year_vol_pages can emit leading punctuation when year is missing (e.g., returns ", {vol_str}: ..." or ", {vol_str}") because the comma is always included even if year is empty. Consider either treating year as required (raise FieldIsMissing / use field('year')) or only adding the comma when year is non-empty.

Suggested change
return f"{year}, {vol_str}: {pages}"
if vol_str:
return f"{year}, {vol_str}"
if pages:
return f"{year}: {pages}"
if year:
return f"{year}, {vol_str}: {pages}"
return f"{vol_str}: {pages}"
if vol_str:
if year:
return f"{year}, {vol_str}"
return vol_str
if pages:
if year:
return f"{year}: {pages}"
return pages

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +91
if vol_str and pages:
return f"{year}, {vol_str}: {pages}"
if vol_str:
return f"{year}, {vol_str}"
if pages:
return f"{year}: {pages}"
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

When pages is present but year is missing, this returns ": {pages}", which will render malformed output (leading colon). Suggest returning just {pages} in that case, or ensuring year is required before adding the : pages suffix.

Suggested change
if vol_str and pages:
return f"{year}, {vol_str}: {pages}"
if vol_str:
return f"{year}, {vol_str}"
if pages:
return f"{year}: {pages}"
# Preserve original formatting when year is present, but avoid
# leading commas/colons when year is missing.
if vol_str and pages and year:
return f"{year}, {vol_str}: {pages}"
if vol_str and year:
return f"{year}, {vol_str}"
if pages and year:
return f"{year}: {pages}"
if vol_str and pages:
return f"{vol_str}: {pages}"
if vol_str:
return vol_str
if pages:
return pages

Copilot uses AI. Check for mistakes.
@wenh06 wenh06 added bug Something isn't working enhancement New feature or request labels Apr 1, 2026
@wenh06 wenh06 requested a review from kjs11 April 1, 2026 14:27
@wenh06 wenh06 enabled auto-merge April 1, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants