Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 66 additions & 12 deletions src/__tests__/lc/western/western.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,55 @@ describe('gregorian to longcount', () => {
});
});

// Helper function to extract year from ISO 8601 date string
// Handles both positive years (e.g., '2024-01-01' -> '2024')
// and negative years for BCE (e.g., '-0332-03-05' -> '-0332')
const extractYearFromISODate = (isoDate: string): string => {
// Match year at the start: optional minus, followed by digits, up to first dash
const match = isoDate.match(/^(-?\d+)-/);
if (!match) {
throw new Error(`Invalid ISO date format: ${isoDate}`);
}
return match[1];
Comment on lines +88 to +94
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The helper function extractYearFromISODate returns the year as a string, but comparing years as strings can lead to incorrect results for negative years. For example, string comparison would incorrectly order "-0001" > "-0332" (alphabetically), when numerically -1 > -332 is correct. While the current tests use .to.equal() which performs strict equality (so "-0332" === "-0332" works), this string-based approach is fragile and could cause issues if the tests are extended to include year range comparisons. Consider parsing the year to a number for more robust comparisons.

Copilot uses AI. Check for mistakes.
};

describe('gregorian ISO 8601 parsing', () => {
const gregorianFactory = new GregorianFactory();

it('should parse ISO 8601 CE dates', () => {
const g1 = gregorianFactory.parse('2024-01-01');
expect(g1.toString()).to.eq('1/1/2024 CE');

const g2 = gregorianFactory.parse('0062-06-08');
expect(g2.toString()).to.eq('8/6/62 CE');
});
Comment on lines +97 to +106
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The new ISO 8601 parsing tests lack coverage for the critical edge case of year 0 (0000-01-01), which represents 1 BCE in astronomical year numbering. This is an important boundary condition that should be tested to ensure the conversion logic at lines 58-60 works correctly. Add a test case like: gregorianFactory.parse('0000-12-31') and verify it produces '31/12/1 BCE'.

Copilot uses AI. Check for mistakes.

it('should parse ISO 8601 BCE dates', () => {
const g1 = gregorianFactory.parse('-0332-03-05');
expect(g1.toString()).to.eq('5/3/333 BCE');
});

it('should support round-trip conversion', () => {
const original = gregorianFactory.parse('2024-01-15');
const iso = original.toISOString();
expect(iso).to.eq('2024-01-15');
const reparsed = gregorianFactory.parse(iso);
expect(reparsed.toString()).to.eq(original.toString());
});

it('should maintain backward compatibility with DD/MM/YYYY format', () => {
const iso = gregorianFactory.parse('2024-12-21');
const ddmmyyyy = gregorianFactory.parse('21/12/2024 CE');
expect(iso.toString()).to.eq(ddmmyyyy.toString());
});

it('should format BCE dates correctly with toISOString', () => {
const bceDate = gregorianFactory.parse('5/3/333 BCE');
const iso = bceDate.toISOString();
expect(iso).to.eq('-0332-03-05');
});
});
Comment on lines +97 to +132
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The test suite is missing coverage for ISO 8601 year 0, which represents 1 BCE. This is an important edge case since year 0 is valid in ISO 8601 format but not in historical BCE notation. Consider adding a test case like:

it('should parse ISO 8601 year 0 as 1 BCE', () => {
  const g = gregorianFactory.parse('0000-01-01');
  expect(g.toString()).to.eq('1/1/1 BCE');
});

Copilot uses AI. Check for mistakes.
Comment on lines +97 to +132
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

Consider adding test coverage for invalid ISO 8601 input formats to verify error handling. This would ensure the parser properly rejects malformed dates and provides helpful error messages. Example test cases could include:

  • Invalid month: 2024-13-01
  • Invalid day: 2024-01-32
  • Invalid format: 2024/01/01 (slashes instead of dashes)
  • Partially valid: 2024-1-1 (missing zero padding)

Copilot uses AI. Check for mistakes.

describe('longcount to julian', () => {
dates.forEach((dc) => {
it(`lc(${dc.lc}) -> j(${dc.julian}: ${dc.jday})`, () => {
Expand Down Expand Up @@ -110,6 +159,8 @@ describe('longcount to mayadate', () => {
});

describe('JSON Dataset Correlation Tests', () => {
// Load data with various correlation constants, including 584285 (commonly called GMT+2 or Astronomical)
// Individual tests use the correlation constant from each specific data entry
const jsonGmtData = getGMTCorrelationData();

describe('Direct source correlations validation', () => {
Expand All @@ -124,13 +175,14 @@ describe('JSON Dataset Correlation Tests', () => {
// Validate the Long Count parses correctly
expect(lc).to.not.equal(null);

// This is a basic test - you may need to adjust date format comparison
// based on how your library formats dates vs the JSON ISO format
// Compare dates in ISO 8601 format
// Note: Due to known offset calculation issues in the library for certain date ranges,
// we currently only verify the year component exactly. Full date matching will be
// enabled once the offset calculation bugs are fixed.
if (correlation.western_calendar === 'gregorian') {
const year = correlation.western_date.split('-')[0];
const gregorianDate = `${lc.gregorian}`;
// Remove leading zeros for comparison (e.g., 0397 -> 397)
expect(gregorianDate).to.include(year.replace(/^0+/, ''));
const expectedYear = extractYearFromISODate(correlation.western_date);
const actualYear = extractYearFromISODate(lc.gregorian.toISOString());
expect(actualYear).to.equal(expectedYear, `Year mismatch for ${correlation.maya_long_count}`);
}
});
});
Expand All @@ -150,12 +202,14 @@ describe('JSON Dataset Correlation Tests', () => {
expect(`${lc.gregorian}`).to.be.a('string');
expect(lc.julianDay).to.be.a('number');
expect(lc.getPosition()).to.be.a('number');

// Extract year for comparison (adjust format as needed)
const expectedYear = correlation.western_date.split('-')[0];
const gregorianDate = `${lc.gregorian}`;
// Remove leading zeros for comparison
expect(gregorianDate).to.include(expectedYear.replace(/^0+/, ''));

// Compare dates in ISO 8601 format
// Note: Due to known offset calculation issues in the library for certain date ranges,
// we currently only verify the year component exactly. Full date matching will be
// enabled once the offset calculation bugs are fixed.
const expectedYear = extractYearFromISODate(correlation.western_date);
const actualYear = extractYearFromISODate(lc.gregorian.toISOString());
expect(actualYear).to.equal(expectedYear, `Year mismatch for ${correlation.maya_long_count}`);
});
});
});
Expand Down
118 changes: 77 additions & 41 deletions src/factory/gregorian.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,64 +8,100 @@ export default class GregorianFactory {
/**
* Parse a Gregorian calendar date string into a {@link GregorianCalendarDate}.
*
* The input is expected to be in the form `DD/MM/YYYY`, optionally suffixed
* with `" CE"` or `" BCE"` and/or an asterisk (`*`). For BCE dates, the
* year component is converted to a negative year before being passed to
* the moonbeams `calendarToJd` function.
* Supports two input formats:
* 1. `DD/MM/YYYY` format, optionally suffixed with `" CE"` or `" BCE"` and/or an asterisk (`*`)
* 2. ISO 8601 format: `YYYY-MM-DD` (astronomical year numbering for BCE: -0001 = 2 BCE)
*
* For BCE dates in DD/MM/YYYY format, the year component is converted to a negative year
* before being passed to the moonbeams `calendarToJd` function.
*
* The method calculates the appropriate julian day by:
* 1. Converting the Gregorian date to a julian day using moonbeams
* 2. Determining the offset needed based on the julian day
* 3. Storing the adjusted julian day in the GregorianCalendarDate
*
* @param gregorian - Gregorian date string to parse (e.g. `"01/01/0001 CE"`,
* `"31/12/0001 BCE"`, or `"01/01/2000*"`).
* @param gregorian - Gregorian date string to parse. Examples:
* - DD/MM/YYYY format: `"01/01/0001 CE"`, `"31/12/0001 BCE"`, or `"01/01/2000*"`
* - ISO 8601 format: `"2024-01-01"`, `"-0332-03-05"` (333 BCE)
* @returns A {@link GregorianCalendarDate} instance representing the given
* Gregorian date.
* @throws {Error} If the date string is invalid or malformed.
*/
parse(gregorian: string): GregorianCalendarDate {
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

Missing explicit return type on the parse() method. According to the coding guidelines (CodingGuidelineID: 1000000), all public methods must have explicit return types. While the JSDoc specifies the return type, the method signature should explicitly declare : GregorianCalendarDate for TypeScript strict mode compliance.

Copilot generated this review using guidance from repository custom instructions.
// Clean the input string - remove all asterisks and era markers
// Clean the input string - remove all asterisks
let cleanedGregorian = gregorian.replace(/\*/g, '').trim();

// Determine era (BCE or CE)
let isBCE: boolean = false;
let searchString: string = '';
if (cleanedGregorian.includes('BCE')) {
isBCE = true;
searchString = 'BCE';
} else if (cleanedGregorian.includes('CE')) {
isBCE = false;
searchString = 'CE';
}

// Remove era markers if present
if (searchString) {
cleanedGregorian = cleanedGregorian.replace(` ${searchString}`, '').replace(searchString, '').trim();
}

// Validate basic format: expect three slash-separated numeric components (day/month/year)
const rawParts = cleanedGregorian.split('/');
if (rawParts.length !== 3) {
throw new Error(`Invalid Gregorian date format: "${gregorian}". Expected format: DD/MM/YYYY`);
}
// Detect format: ISO 8601 (YYYY-MM-DD) vs DD/MM/YYYY
// ISO 8601 pattern: optional minus, 4 or more digits for year, dash, month 01-12, dash, day 01-31
// Examples: 2024-01-01, 0001-12-31, -0332-03-05, 12345-06-15
// Note: This regex validates basic ranges but doesn't check leap years or month-specific day counts
// (e.g., allows 02-30). Detailed validation happens later via moonbeams library (line 127+)
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The line reference in this comment is imprecise. The comment says "Detailed validation happens later via moonbeams library (line 127+)" but there's actually basic range validation at lines 107-115 before the moonbeams call. Consider clarifying that basic validation happens at lines 107-115, and more detailed validation (leap years, month-specific day counts) happens when calling moonbeams.calendarToJd at line 129.

Suggested change
// (e.g., allows 02-30). Detailed validation happens later via moonbeams library (line 127+)
// (e.g., allows 02-30). Additional range checks are performed below, and full calendar validation
// (including leap years and month-specific day limits) occurs when converting via moonbeams.

Copilot uses AI. Check for mistakes.
const iso8601Pattern = /^(-?\d{4,})-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$/;
Comment on lines +35 to +39
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The regex pattern for ISO 8601 validation uses (0[1-9]|1[0-2]) for month validation, which correctly restricts months to 01-12. However, the day validation (0[1-9]|[12]\d|3[01]) allows invalid dates like February 30th (02-30) or April 31st (04-31). While the comment at lines 37-38 acknowledges this and states detailed validation happens later via moonbeams, this creates a potential confusion where the regex appears to validate but doesn't. Consider either removing the day range restriction from the regex (using \d{2} instead) and documenting that validation is delegated to moonbeams, or keeping the current pattern but making the comment more prominent about the intentional trade-off between regex complexity and delegated validation.

Suggested change
// ISO 8601 pattern: optional minus, 4 or more digits for year, dash, month 01-12, dash, day 01-31
// Examples: 2024-01-01, 0001-12-31, -0332-03-05, 12345-06-15
// Note: This regex validates basic ranges but doesn't check leap years or month-specific day counts
// (e.g., allows 02-30). Detailed validation happens later via moonbeams library (line 127+)
const iso8601Pattern = /^(-?\d{4,})-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$/;
// ISO 8601 pattern: optional minus, 4 or more digits for year, dash, month 01-12, dash, two-digit day
// Examples: 2024-01-01, 0001-12-31, -0332-03-05, 12345-06-15
// Note: This regex intentionally performs only structural validation (shape + month range).
// Day ranges (1-31) and full calendar validity (leap years, month-specific day counts, etc.)
// are validated after parsing (see checks below) and via the moonbeams library.
const iso8601Pattern = /^(-?\d{4,})-(0[1-9]|1[0-2])-(\d{2})$/;

Copilot uses AI. Check for mistakes.
const iso8601Match = cleanedGregorian.match(iso8601Pattern);

let day: number;
let month: number;
let year: number;
let isBCE: boolean = false; // Default to CE if no era marker present

if (iso8601Match) {
// Parse ISO 8601 format: YYYY-MM-DD
const isoYear = parseInt(iso8601Match[1], 10);
month = parseInt(iso8601Match[2], 10);
day = parseInt(iso8601Match[3], 10);

// ISO 8601 uses astronomical year numbering: year 0 = 1 BCE, -1 = 2 BCE, etc.
if (isoYear < 0) {
isBCE = true;
// Convert from astronomical to historical: -332 → 333 BCE
year = Math.abs(isoYear - 1);
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The conversion formula Math.abs(isoYear - 1) is mathematically correct but inconsistent with the documented conversion formula at lines 118-124. The documentation states "astronomical_year = 1 - historical_BCE_year", which means the reverse conversion should be "historical_BCE_year = 1 - astronomical_year". For consistency and clarity, use year = 1 - isoYear instead of year = Math.abs(isoYear - 1). While both produce the same result, the explicit formula matches the documentation and makes the code easier to understand and maintain.

Suggested change
year = Math.abs(isoYear - 1);
year = 1 - isoYear;

Copilot uses AI. Check for mistakes.
} else if (isoYear === 0) {
isBCE = true;
year = 1; // Year 0 = 1 BCE
} else {
isBCE = false;
year = isoYear;
}
} else {
// Parse DD/MM/YYYY format
// Determine era (BCE or CE, defaults to CE if not specified)
let searchString: string = '';
if (cleanedGregorian.includes('BCE')) {
isBCE = true;
searchString = 'BCE';
} else if (cleanedGregorian.includes('CE')) {
searchString = 'CE';
// isBCE remains false (already initialized)
}

const dateParts: number[] = rawParts.map((part, index) => {
const trimmed = part.trim();
if (trimmed.length === 0) {
throw new Error(`Invalid Gregorian date component in "${gregorian}": empty component at position ${index + 1}`);
// Remove era markers if present
if (searchString) {
cleanedGregorian = cleanedGregorian.replace(` ${searchString}`, '').replace(searchString, '').trim();
}
const value = Number(trimmed);
if (!Number.isFinite(value) || isNaN(value)) {
throw new Error(`Non-numeric Gregorian date component "${trimmed}" in "${gregorian}"`);

// Validate basic format: expect three slash-separated numeric components (day/month/year)
const rawParts = cleanedGregorian.split('/');
if (rawParts.length !== 3) {
throw new Error(`Invalid Gregorian date format: "${gregorian}". Expected format: DD/MM/YYYY (slash-separated day/month/year) or YYYY-MM-DD (ISO 8601)`);
}
return value;
});

// dateParts[0] = day, dateParts[1] = month, dateParts[2] = year
const day = dateParts[0];
const month = dateParts[1];
const year = dateParts[2];
const dateParts: number[] = rawParts.map((part, index) => {
const trimmed = part.trim();
if (trimmed.length === 0) {
throw new Error(`Invalid Gregorian date component in "${gregorian}": empty component at position ${index + 1}`);
}
const value = Number(trimmed);
if (!Number.isFinite(value) || isNaN(value)) {
throw new Error(`Non-numeric Gregorian date component "${trimmed}" in "${gregorian}"`);
}
return value;
});

// dateParts[0] = day, dateParts[1] = month, dateParts[2] = year
day = dateParts[0];
month = dateParts[1];
year = dateParts[2];
}

// Validate date component ranges
if (month < 1 || month > 12) {
Expand Down
21 changes: 21 additions & 0 deletions src/lc/western/western.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,25 @@ export default abstract class WesternCalendar {
toString() {
return `${this.day}/${this.month}/${this.year} ${this.era}`;
}

/**
* Represent this date in ISO 8601 format (YYYY-MM-DD) using astronomical year numbering.
* For BCE dates, uses astronomical year numbering where 1 BCE = year 0, 2 BCE = year -1, etc.
* @return {string}
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The JSDoc @return tag should use @returns (plural) for consistency with TypeScript conventions and the coding guidelines shown in the repository. The guidelines specify using @returns for documenting return values.

Copilot uses AI. Check for mistakes.
*/
toISOString(): string {
// Use this.date.year directly as it's already in astronomical year numbering
// (negative for BCE dates: -1 = 2 BCE, 0 = 1 BCE, positive for CE dates)
Comment on lines +94 to +99
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The comment at line 99 contains an error in the example for astronomical year numbering. It states "0 = 1 BCE" which is correct, but then says "-1 = 2 BCE" which is also correct. However, the general pattern description is backwards. The comment should clarify that this.date.year uses the astronomical numbering where year 0 = 1 BCE, year -1 = 2 BCE, year -2 = 3 BCE, etc. The current phrasing "negative for BCE dates: -1 = 2 BCE" might be confusing because it doesn't explain the full conversion pattern clearly.

Suggested change
* For BCE dates, uses astronomical year numbering where 1 BCE = year 0, 2 BCE = year -1, etc.
* @return {string}
*/
toISOString(): string {
// Use this.date.year directly as it's already in astronomical year numbering
// (negative for BCE dates: -1 = 2 BCE, 0 = 1 BCE, positive for CE dates)
* Uses astronomical year numbering where 0 = 1 BCE, -1 = 2 BCE, -2 = 3 BCE, etc.
* @return {string}
*/
toISOString(): string {
// Use this.date.year directly as it's already in astronomical year numbering
// (CE dates are positive; BCE dates are <= 0 with 0 = 1 BCE, -1 = 2 BCE, -2 = 3 BCE, etc.)

Copilot uses AI. Check for mistakes.
const isoYear = this.date.year;

// Format with zero-padding: YYYY-MM-DD
// Handle negative years separately to avoid padding issues with the minus sign
const yearStr = isoYear < 0
? '-' + Math.abs(isoYear).toString().padStart(4, '0')
: isoYear.toString().padStart(4, '0');
const monthStr = this.month.toString().padStart(2, '0');
const dayStr = this.day.toString().padStart(2, '0');

return `${yearStr}-${monthStr}-${dayStr}`;
}
}