You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(graph): preserve integrity and temporal evidence - #252
Closes#250. Implements the three approved integrity requirements of parent #232; parent closure remains gated on a separate inventory close-out PR. Start inventory PR #251 is already merged.
Diagnose unresolved occurrence references against the complete input set, independent of file order, with file/field/target/remediation context. Default indexing retains permissive placeholders but emits visible warnings; strict CLI/service validation rejects missing evidence before graph writes or CLI rebuild deletion.
Preserve full deployment observations via deterministic RUNS.eventId, including time/source evidence. Identical replay stays idempotent. Shared deployment query, CLI, HTTP and artifact details expose retained events.
Extend tag history and artifact details with UTC observation-derived validity bounds and explicit ambiguity; order by real instants. Repeated targets share windows, unknown ends stay null, and latest conflicting observations no longer arbitrarily resolve to one digest.
Reject old derived RUNS schemas with explicit rebuild guidance; no source-schema migration or fabricated recovery of missing history.
Add read-only graph CI, regressions, configuration support through existing Helm config, and architecture/user documentation. No chart defaults or registry fixture workflows changed.
Validation
Testing mode: hosted, normal GitHub Actions and Copilot review authorized.
Tested source: 25b68e6c2caea50ece38f59299708c88619d6e9b on Python 3.12/macOS arm64 with the real LadybugDB engine.
Self-review: assessed correctness, reference-policy boundaries, append-only/idempotent identity, timestamp ambiguity, old-schema behavior, user-facing contracts and safety; no remaining findings at the tested head. Source-documentation templates/editor diagnostics are unavailable, so repository docstrings, CLI type/syntax checks and execution tests ground the assessment.
The approved normal merge will trigger existing dashboard-image publishing and graph-event recording. No manual deployment or registry-fixture runs are requested.
The same 123 local tests and all 8 production-file type checks pass at this exact head. Syntax checks, workflow diagnostics, seven relative docs links, and baseline-aware Ruff comparison remain clean for this change.
All five initial review threads have individual fix-SHA replies and were resolved afterward. Fresh current-head Copilot review remains a merge gate.
Timestamp handling now preserves exact RFC3339 fractions, lowercase t/z and leap-second ordering without narrowing schema acceptance.
Deployment history retains the complete source payload, not just its hash/run URL. Queries return the source evidence, with HTTP regressions for runAttempt/issueUrl/workflow distinctions.
Database opens, including read-only CLI commands, reject legacy history schemas with actionable rebuild guidance. No migration or source-file changes are hidden in that guard.
No hosted CI failure or retry-cap escalation so far. The earlier baseline evidence is historical and superseded by this current head.
Copilot completed exact-head review and recommended approval, with no active issues: https://github.com/toddysm/cssc-framework/actions/runs/40749989409. All inline threads have fix-SHA replies before resolution; summary-only concerns received individual fix/disposition comments.
146 local tests, ten production-file type checks, syntax checks, workflow/schema and ShellCheck diagnostics passed. Seven documentation links resolve; no new Ruff diagnostics versus main; final safety scan/self-review has no remaining findings. Existing unrelated Starlette deprecation/style findings remain disclosed.
Corrections from review enforce a shared timestamp-validation/query contract, exact fractions and valid offset/leap structure; preserve full deployment source evidence; surface legacy/startup diagnostics; and batch history retrieval into two database calls including seed resolution, measured against10 occurrences.
Documented evidence limits: timestamp structure is not an external announcement/event verifier; inferred tag windows are not proof of continuous registry validity; full-history queries retain the safety read guard and are not a paginated inventory API.
No runtime dependency manifests or chart defaults changed. No manual deployment/registry fixture actions. No hosted CI failures, bypasses, fabricated statuses or protection changes.
This final evidence supersedes the earlier historical source-head counts. Parent #232 stays open until the separate inventory close-out PR merges.
Accept schema-valid lowercase UTC and leap-second notation, retaining arbitrary fractional precision for tag grouping, bounds and deployment ordering.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This PR strengthens the file-backed supply-chain graph with reference diagnostics and observation history, addressing #250 while leaving #232’s inventory close-out separate.
Persist complete deployment source payloads in the derived graph and expose them through shared/HTTP history. Reject incompatible schemas at read-only opens with rebuild guidance, without silently migrating the database.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On startup, create_app catches this exception and logs only its traceback; IndexingError's message contains the error count, not these diagnostics. Since /readyz also returns only a generic 503, a strict-reference failure leaves the service unready without emitting the file, field, target, or remediation anywhere in startup output. Please log the formatted diagnostics before raising, so operators can identify the missing evidence without invoking POST /index/rebuild.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Compatibility checks can produce an uncaught CLI traceback for older databases, and strict-reference startup failures lack actionable diagnostics identifying missing evidence and remediation.
On a strict-reference startup failure, the logged exception reports only the validation error count, while /readyz returns a generic 503. Operators cannot identify the missing evidence from either surface without calling POST /index/rebuild. Log the formatted file, field, target, and remediation diagnostics before raising so startup failures are actionable.
Catch legacy compatibility failures when entering the index context and log complete strict-reference diagnostics before startup rejection. Cover both paths with execution regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixed the additional startup-diagnostics concern from the latest review summary in d52d45f. GraphIndex logs every formatted file/field/target/remediation diagnostic before raising the startup error; the strict-service regression captures the logs and asserts all four details. The user guide documents this operator-facing error path. This summary concern had no inline thread to resolve. All 124 local graph/HTTP tests and eight-file type checks pass at this head.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed to evaluate a timestamp-validation gap that can admit malformed observations and cause HTTP 500 responses across multiple read endpoints.
Register the shared exact RFC3339 parser as the schema date-time checker so optional jsonschema extras cannot silently disable validation. Reject malformed timestamps before graph writes or query-time failures, with CLI/service regressions and no runtime dependency changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixed the latest review summary’s timestamp-validation gap in ce0facc. I reproduced the actual root cause: the installed jsonschema FormatChecker had no date-time checker because optional format extras were absent, so even not-a-timestamp was accepted.
Schema validation and queries now share one exact RFC3339 parser, explicitly registered as the date-time checker without new runtime dependencies. Malformed timestamps are rejected with file/field diagnostics before indexing; accepted lowercase UTC, exact fractions and leap notation remain queryable. New CLI/schema/service regressions prove failures occur before DB creation/readiness rather than at read endpoints. All 132 graph/HTTP tests and ten production-file type checks pass. This was a summary-only concern without an inline thread.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The timestamp parser accepts and normalizes invalid timezone offsets, allowing malformed evidence through schema validation and potentially creating false simultaneous tag conflicts.
Enforce RFC3339 offset hour/minute ranges before datetime parsing can normalize invalid offsets. Cover malformed offsets and both valid signed boundary values.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject impossible leap-second placement after timezone normalization, retain real historical leap notation, and document the distinction between structural timestamp validation and external event/schedule verification.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressed the latest review summary’s leap-marker assessment in 41810b2. The parser now rejects leap notation unless the UTC-normalized time is23:59 at the last day of a month. New invalid-placement tests include midday, wrong minute/day and an offset that moves the marker away from UTC midnight; valid tests use the real 2016-12-31 leap and its offset-equivalent spelling. All145 graph/HTTP tests and ten production-file type checks pass.
The guide now states the remaining evidence boundary explicitly: schema validation checks timestamp structure, not whether an external authority announced an insertion or whether the recorded event actually occurred. No remote schedule lookup or unverifiable future-event attribution is introduced. This summary concern had no inline thread.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed to evaluate the scalability and request-blocking risks of per-occurrence deployment queries executed while holding the graph lock.
Fetch all resolved occurrence history in one parameterized projection instead of a per-occurrence query loop, while retaining the rebuild read guard and full evidence. Add a real-engine constant-query-count regression.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assessed and addressed the latest summary’s per-occurrence query/lock concern in f2b2478. Deployment history now uses one parameterized graph projection for all resolved occurrences instead of N database round-trips. A real-engine regression with10 occurrences measures exactly2 calls (seed resolution + one history query). The existing read guard remains necessary to prevent concurrent rebuilds closing the store mid-query.
The guide explicitly records the remaining boundary: this history journey returns retained evidence and is not a streaming/paginated inventory API, so cost scales with returned history. That matches the existing history-query contract; no silent truncation or new unapproved pagination semantics were introduced. All146 graph/HTTP tests and ten production-file type checks pass. No unresolved inline findings remain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Scope
Closes #250. Implements the three approved integrity requirements of parent #232; parent closure remains gated on a separate inventory close-out PR. Start inventory PR #251 is already merged.
Validation
Testing mode: hosted, normal GitHub Actions and Copilot review authorized.
Tested source:
25b68e6c2caea50ece38f59299708c88619d6e9bon Python 3.12/macOS arm64 with the real LadybugDB engine.python -m pytest --import-mode=importlib -q apps/python-app/libs/cssc_graph/tests apps/python-app/services/graph-service/tests: 117 passed, no skips (87-test baseline; 30 added cases).mypy --python-executable .venv/bin/python --follow-imports=silentwith library path configured: 8 changed production files clean.Self-review: assessed correctness, reference-policy boundaries, append-only/idempotent identity, timestamp ambiguity, old-schema behavior, user-facing contracts and safety; no remaining findings at the tested head. Source-documentation templates/editor diagnostics are unavailable, so repository docstrings, CLI type/syntax checks and execution tests ground the assessment.
The approved normal merge will trigger existing dashboard-image publishing and graph-event recording. No manual deployment or registry-fixture runs are requested.
Current-head evidence after review corrections
Tested/pushed source:
b226351304bdcf9f489d3fc2b984a85614b31344.Final validated head and review disposition
Tested/pushed source:
f2b2478617b7302cc2544eabca8d86306409c46b.This final evidence supersedes the earlier historical source-head counts. Parent #232 stays open until the separate inventory close-out PR merges.