build: harden the initial Saju CalDAV release - #6
Conversation
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current heada565ea15e33b5175a473baba6f5f181ed805371a. -
Head SHA:
a565ea15e33b5175a473baba6f5f181ed805371a -
Workflow run: 29725987279
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 2
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: ci.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: ci.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (53 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (53 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (6 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (6 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (9 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (9 files)"]
R4 --> V4["targeted test run"]
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Hardens the initial Saju CalDAV release by adding solar/Korean-lunar birth input normalization, city-based timezone/true-solar handling, privacy-minimized iCalendar publishing with visibility mapping, and tighter deployment/CI controls.
Changes:
- Add lunar (incl. leap-month) birth input normalization and persist original input components alongside normalized Gregorian wall time.
- Introduce curated birth-city presets to resolve IANA timezone and keep coordinates off the client while supporting true-solar mode.
- Harden CalDAV publishing (httpx + URL validation, CLASS mapping, minimal VEVENT content) and container/CI dependency locking.
Reviewed changes
Copilot reviewed 66 out of 69 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_store.py | Updates round-trip tests for new profile/calendar fields and adds legacy migration/backfill coverage. |
| tests/test_saju.py | Switches to synthetic public example chart and adds Pillar Korean explanation + ipchun boundary test. |
| tests/test_rules.py | Updates acceptance fixtures for new synthetic chart and rule literal symbol. |
| tests/test_locations.py | Adds tests for city catalog privacy and timezone/longitude resolution behavior. |
| tests/test_events.py | Updates window-generation tests for new synthetic example and default-range behavior. |
| tests/test_deployment.py | Asserts Dockerfile uses hashed lock installs and vendor copy behavior. |
| tests/test_caldav.py | Adds CalDAV base URL validation test and visibility→CLASS mapping assertions. |
| tests/test_birth.py | Adds unit tests for solar vs Korean lunar normalization and leap-month validation. |
| tests/test_api.py | Updates API integration tests for new profile inputs, locations endpoint, visibility, and default date ranges. |
| scripts/private_regression.py | Adds opt-in private regression runner driven by env vars without logging inputs. |
| scripts/acceptance_smoke.py | Migrates smoke script to httpx, adds visibility checks, and avoids committing private fixtures. |
| pyproject.toml | Replaces lunar conversion dep, packages vendored lunar_python, and excludes it from ruff. |
| lunar_python/util/init.py | Vendored lunar-python package init (MIT) for offline reproducibility. |
| lunar_python/util/TaoUtil.py | Vendored lunar-python utility module. |
| lunar_python/util/SolarUtil.py | Vendored lunar-python utility module. |
| lunar_python/util/FotoUtil.py | Vendored lunar-python utility module. |
| lunar_python/eightchar/init.py | Vendored lunar-python eightchar package init. |
| lunar_python/eightchar/Yun.py | Vendored lunar-python eightchar module. |
| lunar_python/eightchar/XiaoYun.py | Vendored lunar-python eightchar module. |
| lunar_python/eightchar/LiuYue.py | Vendored lunar-python eightchar module. |
| lunar_python/eightchar/LiuNian.py | Vendored lunar-python eightchar module. |
| lunar_python/eightchar/DaYun.py | Vendored lunar-python eightchar module. |
| lunar_python/init.py | Vendored lunar-python top-level exports. |
| lunar_python/TaoFestival.py | Vendored lunar-python model. |
| lunar_python/Tao.py | Vendored lunar-python module. |
| lunar_python/SolarYear.py | Vendored lunar-python module. |
| lunar_python/SolarWeek.py | Vendored lunar-python module. |
| lunar_python/SolarSeason.py | Vendored lunar-python module. |
| lunar_python/SolarMonth.py | Vendored lunar-python module. |
| lunar_python/SolarHalfYear.py | Vendored lunar-python module. |
| lunar_python/Solar.py | Vendored lunar-python core solar date implementation. |
| lunar_python/ShuJiu.py | Vendored lunar-python module. |
| lunar_python/NineStar.py | Vendored lunar-python module. |
| lunar_python/LunarYear.py | Vendored lunar-python module. |
| lunar_python/LunarTime.py | Vendored lunar-python module. |
| lunar_python/LunarMonth.py | Vendored lunar-python module. |
| lunar_python/JieQi.py | Vendored lunar-python module. |
| lunar_python/Holiday.py | Vendored lunar-python module. |
| lunar_python/Fu.py | Vendored lunar-python module. |
| lunar_python/FotoFestival.py | Vendored lunar-python model. |
| lunar_python/Foto.py | Vendored lunar-python module. |
| lunar_python/EightChar.py | Vendored lunar-python core EightChar implementation. |
| licenses/lunar-python-MIT.txt | Adds MIT license text for vendored lunar-python. |
| docs/superpowers/specs/2026-07-19-saju-caldav-design.md | Updates design spec to lunar input, privacy posture, and default matching windows. |
| docs/superpowers/plans/2026-07-19-saju-caldav.md | Updates plan to remove committed private fixtures and use synthetic public tests. |
| docs/research/README.md | Documents lunar conversion and vendored lunar-python rationale and bounds. |
| docs/DEPLOYMENT.md | Updates smoke/regression guidance and classification semantics. |
| docs/ARCHITECTURE.md | Updates architecture and data flow for locations and lunar normalization. |
| app/store.py | Adds new schema columns, migration/backfill, and calendar visibility persistence. |
| app/static/styles.css | UI styling for new inputs (checkbox, advanced settings, pillar disclosures). |
| app/static/index.html | Reworks UI for solar/lunar inputs, city selection, and visibility selection. |
| app/static/app.js | Adds locations fetch/rendering, Korean-first labels, and default range preview/sync behavior. |
| app/saju.py | Adds Korean explanations for stems/branches and exposes them on Pillar. |
| app/main.py | Adds birth normalization + place resolution, locations endpoint, and default date range resolution. |
| app/locations.py | Introduces server-side birth-city catalog + resolution logic without client coordinates. |
| app/caldav.py | Switches to httpx, validates base URL, adds visibility→CLASS mapping, and strips sensitive VEVENT fields. |
| app/birth.py | Adds solar/Korean lunar normalization using korean-lunar-calendar. |
| THIRD_PARTY_NOTICES.md | Adds third-party notice entry for vendored lunar-python. |
| SECURITY.md | Adds security reporting policy and sensitive-data guidance. |
| README.md | Updates product description, features, and private regression workflow. |
| Dockerfile | Pins base image digest and installs from hashed requirements.lock; vendors lunar_python. |
| .github/workflows/ci.yml | Pins GitHub Actions by commit SHA for supply-chain hardening. |
Comments suppressed due to low confidence (3)
lunar_python/Solar.py:1
- The day-range validation is incorrect: it checks
month > 31instead ofday > 31, which means invalid day values (e.g., 32) won’t be rejected. Even though this is vendored code, this is a concrete logic error that can propagate invalid dates into downstream calculations. Fix the condition to validateday > 31(and consider keeping a minimal patch note if you’re intentionally tracking upstream behavior).
scripts/acceptance_smoke.py:1 - The helper defaults
Content-Typetoapplication/json, but CalDAV/XML methods likePROPFINDshould consistently sendapplication/xml(you do pass it correctly here). To prevent future call sites from accidentally sending JSON content-types for WebDAV methods, consider changing the defaultcontent_typeto something neutral (or requiring it explicitly whenbodyis provided), since mixing content-types can break against stricter CalDAV servers.
scripts/acceptance_smoke.py:1 - The helper defaults
Content-Typetoapplication/json, but CalDAV/XML methods likePROPFINDshould consistently sendapplication/xml(you do pass it correctly here). To prevent future call sites from accidentally sending JSON content-types for WebDAV methods, consider changing the defaultcontent_typeto something neutral (or requiring it explicitly whenbodyis provided), since mixing content-types can break against stricter CalDAV servers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 66 out of 69 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
lunar_python/Solar.py:1
- The day-range validation is incorrect:
if day < 1 or month > 31should checkday > 31, notmonth > 31. Even if current app usage only constructsSolarfrom validdatetimevalues, this is a concrete logic error in the vendored code and can cause invalid dates to slip through or valid ones to be rejected in other call paths. Patch the vendored source or add a minimal local guard before any construction paths that might accept untrusted Y/M/D inputs.
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head422cbc8b5e482fa5853ed6173e53ca7a4e5aec74. -
Head SHA:
422cbc8b5e482fa5853ed6173e53ca7a4e5aec74 -
Workflow run: 29919524888
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 2
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: ci.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: ci.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file (54 files)"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file (54 files)"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (6 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (6 files)"]
R3 --> V3["docs review"]
Evidence --> S4["Test (9 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (9 files)"]
R4 --> V4["targeted test run"]
Coverage-tooling review superseded: the project-pinned environment now passes the complete 49-test suite and Ruff, current-head CI/container/security checks are clean, and no source-scoped finding remains.
Summary
Current HEAD verification
422cbc8b5e482fa5853ed6173e53ca7a4e5aec74uv run --frozen python -m pytest -q: 49 passeduv run --frozen ruff check .: passednode --check app/static/app.js: passeduv lock --check: passedMerge
The pull request remains head-guarded by the organization review policy. Auto-merge will be enabled after a valid exact-current-head automated approval replaces the previous-head change request.