fix(skill): traffic template, open-is-a-status, k8s terms, event scope - #35
Conversation
From the v0.3.5 40-attempt rerun, where every remaining flag was interpretation rather than data: - Traffic claims were still collapsing eligibility, observed activity and current success. Three rounds of prohibitions have not landed, so the skill now gives the required shape instead: endpoint-eligible; recent APM activity (last observed T); current successful traffic unverified. - A date-bounded PagerDuty query hid an older open incident. Open is a status, never a time window; a 24-hour filter reads as "no open incident" for the service currently paging someone. - A PR review described a memory limit as a scheduling reservation. Requests are what the scheduler reserves, limits are the ceiling an OOM kill is measured against; the graph carries both, so the answer names which it read. - An OOM event was cited without its namespace. The same event type fires in every namespace of every cluster, so events are cited with their scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2dcfde3c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - Cite `hashedID`s and event timestamps for every load-bearing claim. For EVENTS also cite | ||
| the namespace and cluster: the same event type (`pod_oom_killed`, `deployment_degraded`) | ||
| fires in every namespace of every cluster, so an event quoted without its scope is how a | ||
| real event ends up attached to the wrong workload. |
There was a problem hiding this comment.
Requiring namespace and cluster for every EVENT is impossible for project-wide events that do not have those fields. For example, the repository's deploy-impact recipe includes the provider event update_function_code, while cluster-scoped Kubernetes events such as node_preempted also have no namespace. In those cases this instruction forces the agent either to invent scope or suppress valid evidence; require these fields only when present and use the event's target identifier/source for other event kinds.
Useful? React with 👍 / 👎.
| - **Succeeding now**: requests returning success at this moment. NOT in the graph — only a | ||
| live APM query or the native source. If you did not run one, the third clause stays | ||
| "unverified", whatever the first two say. |
There was a problem hiding this comment.
When the mounted native source is only the Kubernetes MCP described above, this wording permits endpoint or pod state to verify that requests are currently succeeding, even though those APIs expose readiness rather than successful responses. That collapses the eligibility and success facts the new section is intended to separate; current success should require request-level telemetry carrying outcomes or an active request probe, not merely any native source.
Useful? React with 👍 / 👎.
… is not success (codex) - Requiring namespace and cluster on every event forces invention or suppression for cluster-scoped events (node_preempted) and provider events (update_function_code) that carry neither. Cite the scope the event actually has, and the target identifier plus source otherwise. - The success clause allowed any mounted native source to verify it, so a Kubernetes read showing readiness could stand in for requests succeeding, collapsing the two facts the section separates. Current success now requires request-level evidence carrying outcomes, or a probe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd6ee6ccae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| > Endpoint-eligible; recent APM activity (last observed T); current successful traffic unverified. | ||
|
|
||
| - **Eligible**: a Service selects a ready endpoint. Structural, from the graph. |
There was a problem hiding this comment.
When the cluster agent has stopped reporting or the latest EndpointSlice observation is stale, this graph-only rule makes the template report Endpoint-eligible as a current fact even though the graph proves only that the endpoint was ready at the last shipped snapshot. This contradicts the same file's requirement to time-stamp graph state and its statement that EXPOSES {ready} reflects the last observed EndpointSlice; require native Kubernetes verification for current eligibility or phrase it as readiness at the last snapshot.
Useful? React with 👍 / 👎.
…me (codex)
The template asserted Endpoint-eligible as a bare present fact while the same
file requires graph state to be time-stamped and says EXPOSES {ready} is the
readiness at the last EndpointSlice observed. A cluster whose agent stopped
reporting freezes that value, so the clause now carries its time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@lerenn one more skill-text PR, three Codex rounds addressed, checks green. Blocking a v0.3.6 release for the benchmark rerun. |
2 participants
From Roxane's v0.3.5 rerun (40 attempts; plugin arm 102/120 coverage vs 95/120 native, 1 critical vs 2). Every remaining flag was interpretation, not data — the data bug she found is backend #1380.
Endpoint-eligible; recent APM activity (last observed T); current successful traffic unverified— her own stated pass condition, with each clause defined and the negative case covered.status, never a time window: an incident opened three weeks ago can still betriggered, and a 24-hour filter reads as "no open incident" for the service currently paging someone.readyvsphase,replicasvs live pods.Not addressed here, deliberately: the plugin called Kubernetes in 13/20 runs but in none of the six incident/inventory runs. That is the third attempt at the same instruction and wording is no longer the lever — see the PR discussion.
🤖 Generated with Claude Code