Add per-tip identity, surface, and combine controls + tip_suggestion hook#1202
Merged
Conversation
✨ Aspect Workflows Tasks📅 Thu Jun 4 19:57:50 UTC 2026
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d24d4ed3f9
ℹ️ 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".
0066317 to
f9a5506
Compare
ec1d98d to
91c1102
Compare
91c1102 to
2f08e20
Compare
308fa88 to
4848631
Compare
…suggestion hook Reworks the tips framework around a small, orthogonal set of controls. Identity: a tip is `(id, key)`. `id` is the kind + silence key (`--tips:silence=<id>` covers every key); an optional `key` makes same-`id` tips distinct so "same tip, different subject" coexists as separate rows. A re-emit of the same `(id, key)` replaces the stored tip, with `accumulate` fields unioned (deduped + sorted) onto the prior. Inputs: `vars` are fixed scalars (FORMAT interpolation / JINJA2 constants); `accumulate` are list-valued JINJA2 fields that union on re-emit and, when `combine_across_tasks`, across tasks. A name in both is an error. Surfaces: a `surfaces` allowlist (SURFACE_CLI / _GHSC / _BK / _GITLAB / _PR_SUMMARY, plus TASK_SCREENS / AGGREGATE_SCREENS / SURFACE_ALL shorthands; empty = all) gates which screens render a tip, via the shared `tip_shows_on` predicate; the CLI print is gated on SURFACE_CLI. Omitting SURFACE_PR_SUMMARY keeps a tip off the rollup. PR summary: same-`(id, key)` tips combine by `combine_across_tasks` — True unions `accumulate` across tasks into one row; False shows the latest task's content (identical content across tasks is attributed to all). The built-in add-github-token-scope tip sets combine_across_tasks. tip_suggestion hook: a `TipsTrait.tip_suggestion` hook fires on every add_tip with a TipInfo and returns TIP_ACCEPT / TIP_REJECT / tip_replace(...), letting config.axl veto or rewrite any tip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4848631 to
cfd81b2
Compare
gregmagolan
added a commit
that referenced
this pull request
Jun 4, 2026
A worked example of the per-tip `surfaces` controls added in #1202: a custom `suggestion` tip that surfaces click-through links on every build/test task. Registered inline in `config(ctx)` by subscribing to the `task_update` lifecycle event — no new framework hook. The handler reads three links off each `TaskUpdate` the way the built-in surfaces do and shows whichever it knows, one per line in the built-in metadata-row style: ``` ✨ Aspect Web UI — invocation details, test logs, build logs, and timing 🔗 BES results 🐙 GitHub Actions — raw CI logs for this job ``` - **Aspect Web UI** — `aspect_web_ui_url(env, data)` (`sink_invocation_id` joined onto `ASPECT_WORKFLOWS_BES_RESULTS_URL`). - **BES results** — `render_bes_url` (Bazel's `invocation_id` + `bes_results_url`). - **CI build** — `detect_build_url` + the per-host icon/label from `detect_ci_host` (🐙 GitHub Actions, ⭕ CircleCI, 🏗 Buildkite, 🦊 GitLab CI). Each emit rebuilds the full body from whatever links have resolved and re-adds the tip under a stable id; the default last-emit-wins replace overwrites the prior tip in place, so it grows as links appear across separate `task_update` emits. `surfaces = TASK_SCREENS` keeps these per-invocation links off the cross-task PR summary. It calls the single `add_tip(ctx, id=…, severity=…, type=…, surfaces=…, title=…, body=…)` producer API (from #1209) — the example was updated alongside the API collapse from `emit_tip(ctx, {…})` to a single named-arg `add_tip`. Stacked on #1209 (`tips-public-facades`); the diff is `.aspect/config.axl` only once that lands. --- ### Changes are visible to end-users: no <!-- example config in this repo's own .aspect/, not a shipped @aspect builtin --> - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no ### Test plan - Manual: `ASPECT_CLI_TESTING=github_status_checks aspect build //examples/lint/...` surfaces the tip with the GitHub Actions link bar, deduped across the build's `task_update` emits; `buildifier` clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds per-tip control over identity, which status screens a tip renders on, and how same-tips from different tasks combine in the PR summary, plus a hook to intercept tip creation.
Identity —
(id, key)A tip is identified by
(id, key):id— the tip kind + silence key.--tips:silence=<id>(and thesilencefeature arg) match onid, so silencing one id covers every key under it.key— optional discriminator (default""). Sameid, differentkey⇒ distinct tips that coexist on a task's surface and as separate PR-summary rows. Use it for "same tip kind, different subject" (e.g. oneflaky-testtip per failing target, all silenceable with--tips:silence=flaky-test).A re-emit of the same
(id, key)within a task replaces the stored tip;accumulatefields union (deduped + sorted) onto the prior.Inputs —
varsvsaccumulatevars— fixed scalars:str.format-interpolated forTIP_TEMPLATE_FORMAT, exposed as scalars toTIP_TEMPLATE_JINJA2templates.accumulate— list-valued JINJA2 fields: a template sees{{ name }}as a deduped + sorted list; values union on re-emit and (whencombine_across_tasks) across tasks.A name set in both is an error.
Surfaces
A
surfacesallowlist names the screens a tip may render on:SURFACE_CLI,SURFACE_GITHUB_STATUS_CHECKS,SURFACE_BUILDKITE_ANNOTATIONS,SURFACE_GITLAB_COMMIT_STATUSES,SURFACE_GITHUB_PR_SUMMARY(or theTASK_SCREENS/AGGREGATE_SCREENS/SURFACE_ALLshorthands; empty ⇒ all).collect_tips_sorted(ctx, surface=…)filters via the sharedtip_shows_onpredicate; the CLI print is gated onSURFACE_CLI. OmittingSURFACE_GITHUB_PR_SUMMARYkeeps a tip off the cross-task rollup.Cross-task combination —
combine_across_tasksFor same-
(id, key)tips from sibling tasks in the PR summary:True— union theaccumulatefields across every task into one row (e.g. every scope every task was missing).False(default) — one row with the latest task's content (identical content across tasks is attributed to all of them).The built-in
add-github-token-scopetip setscombine_across_tasksto union scopes/endpoints across tasks.tip_suggestionhookA
TipsTrait.tip_suggestionhook fires on everyadd_tip, receives aTipInfo, and returnsTIP_ACCEPT/TIP_REJECT/tip_replace(...)— lettingconfig.axlveto or rewrite any tip (suppress an id, downgrade a severity, re-scopesurfaces). Same accept/reject/replace shape as the existingrepro_fix_suggestionhook.Changes are visible to end-users: yes
Suggested release notes
(id, key)identity (distinctkeys coexist; silencing is byid), asurfacesallowlist controlling which status screens render a tip, acombine_across_tasksflag controlling cross-task PR-summary union, separatevars(fixed) /accumulate(list-valued) template inputs, and atip_suggestionhook onTipsTraitto accept / reject / rewrite any tip from.aspect/config.axl.Test plan
tips_test.axl—(id, key)identity (re-emit replaces; distinct key coexists; silence-by-id across keys); within-taskaccumulateunion (seed / merge / dedup / partial-overlap);vars+accumulatemix and vars-only JINJA2 re-emit;surfaces/combine_across_tasksstorage + defaults;emit_tiptemplate-vs-arg precedence;tip_shows_on; per-surface collect filter;tip_to_payload; thetip_suggestionhook (accept / reject / reject-by-id / replace / chain short-circuit).github_status_comments_test.axl— surface gate;(id, key)grouping;combine_across_tasksunion; latest-wins (identical → all tasks, divergent → latest); distinct-key separate rows; combine-if-any-contributor-opts-in.