Frontend triage severity as comment suggestion

agents/frontend-triage/README.md

@@ -132,9 +132,8 @@ Two caveats before acting on a plan:
 ## What it writes back to Bugzilla
 
 **Nothing, during a run.** `ENABLED_ACTION_TYPES` in `config.py` allows
-`bugzilla.add_comment` and `bugzilla.update_bug`, but those come from an
-in-process actions server that appends to `summary.json` and makes no network
-calls. The only Bugzilla access the agent has is through the broker sidecar,
+`bugzilla.add_comment` and nothing else, and that tool comes from an in-process
+actions server that appends to `summary.json` and makes no network calls. The only Bugzilla access the agent has is through the broker sidecar,
 which exposes five read tools and holds the API key.
 
 **Afterwards, though, a confident run posts itself.** When the run reports
@@ -152,10 +151,12 @@ hackbot-api applies whatever it finds in `summary.json`, dispatching it against
 handler registry far wider than the tools this agent was given.
 
 - `add_comment_hook` — one comment, public, on the bug being triaged.
-- `update_bug_hook` — one field change, add-only, on the bug being triaged, and
-  only `keywords`/`severity` with values from `TRIAGE_SEVERITIES` /
-  `TRIAGE_KEYWORDS` in `config.py`. Widen those sets alongside the rule that needs
-  the new value.
+
+That is the whole list, because a comment is the only thing this agent can write.
+It has no tool that changes a bug's fields: `severity` was the one field a ruleset
+directed it to set, and that is now a suggestion at the end of the comment for a
+human to apply, so `bugzilla.update_bug` left `ENABLED_ACTION_TYPES` rather than
+staying on with no caller. `tests/test_config.py` guards that.
 
 A refusal reaches the agent as a tool error it can correct in the same run, and the
 action never lands in `summary.json`. The action _type_ needs no check:
@@ -182,9 +183,12 @@ reactions and tags are the feedback channel — the agent does not request needi
 
 A run that applies itself reports two lines to the channel of the team that owns
 the bug's component: the bug, linked, with the run's one-line summary, and a link
-to the run. An `S1` severity assessment adds a `:red_circle:` and names the level.
-Nothing else — the analysis is on the bug, the detail is in the run, and the
-channel already says which component this is.
+to the run. An `S1` the run is confident about adds a `:red_circle:` and names the
+level as `(suggested S1)` — suggested, because nothing was written to the field.
+Below `REPORTABLE_SEVERITY_CONFIDENCES` there is no marker, matching the comment,
+which omits its severity block on the same threshold. Nothing else — the analysis
+is on the bug, the detail is in the run, and the channel already says which
+component this is.
 
 The audience is the team whose bug was just written to by nobody, so only an
 auto-applied run notifies. A medium or low result wrote nothing to Bugzilla and

agents/frontend-triage/hackbot_agents/frontend_triage/agent.py

@@ -41,7 +41,7 @@
     permalink_prefix,
     resolve_index_revision,
 )
-from pydantic import BaseModel, ValidationError
+from pydantic import BaseModel
 from searchfox import AsyncSearchfoxClient
 
 from .config import (
@@ -50,9 +50,10 @@
     MOZILLA_VCS_TOOLS,
     SEARCHFOX_TOOLS,
     TRIAGE_SCOPE,
+    TRIAGE_SEVERITIES,
     ScopedComponent,
 )
-from .hooks import add_comment_hook, update_bug_hook
+from .hooks import add_comment_hook, severity_block_hook
 
 HERE = Path(__file__).resolve().parent
 
@@ -237,6 +238,35 @@ def parse_confidence(value: object) -> str | None:
     return normalized if normalized in CONFIDENCE_LEVELS else None
 
 
+def parse_severity(value: object) -> str | None:
+    """One of :data:`~.config.TRIAGE_SEVERITIES`, or None if ``value`` isn't one.
+
+    Same contract as :func:`parse_confidence`, for the same reason: the level comes out
+    of the agent's free-form JSON block, and it is the only severity signal reaching a
+    human now that the field is no longer written.
+    """
+    if not isinstance(value, str):
+        return None
+    normalized = value.strip().upper()
+    return normalized if normalized in TRIAGE_SEVERITIES else None
+
+
+def parse_severity_assessment(value: object) -> SeverityAssessment | None:
+    """A :class:`SeverityAssessment` with its level and confidence normalized.
+
+    Fields degrade independently: an unreadable level or confidence becomes None, which
+    drops the comment's severity block, rather than discarding the rationale with it.
+    """
+    if not isinstance(value, dict):
+        return None
+    rationale = value.get("rationale")
+    return SeverityAssessment(
+        suggested=parse_severity(value.get("suggested")),
+        confidence=parse_confidence(value.get("confidence")),
+        rationale=rationale if isinstance(rationale, str) else None,
+    )
+
+
 def may_apply_unattended(plan: dict) -> bool:
     """Whether this run's recorded actions may reach the bug without a human.
 
@@ -284,14 +314,6 @@ def _as_str(value):
         # run's result, and these two only route a notification.
         return value.strip() or None if isinstance(value, str) else None
 
-    def _as_model(model, value):
-        if not isinstance(value, dict):
-            return None
-        try:
-            return model.model_validate(value)
-        except ValidationError:
-            return None
-
     actionable = data.get("actionable")
     if not isinstance(actionable, bool):
         actionable = None
@@ -306,8 +328,8 @@ def _as_model(model, value):
         "actionable": actionable,
         "regressor_node": data.get("regressor_node"),
         "relevant_tests": _as_list(data.get("relevant_tests")),
-        "severity_assessment": _as_model(
-            SeverityAssessment, data.get("severity_assessment")
+        "severity_assessment": parse_severity_assessment(
+            data.get("severity_assessment")
         ),
     }
 
@@ -369,9 +391,7 @@ async def run_frontend_triage(
     actions_recorder.add_hook(
         "bugzilla.add_comment", add_comment_hook(actions_recorder, bug)
     )
-    actions_recorder.add_hook(
-        "bugzilla.update_bug", update_bug_hook(actions_recorder, bug)
-    )
+    actions_recorder.add_hook("bugzilla.add_comment", severity_block_hook)
 
     actions_recorder.add_hook(
         "bugzilla.add_comment",

agents/frontend-triage/hackbot_agents/frontend_triage/config.py

@@ -31,17 +31,15 @@
 ]
 
 
-# Recordable action types the agent may take, by dotted id. This agent triages
-# and plans only: it records a comment with its findings/plan and, at high
-# confidence, may propose field updates (e.g. keyword/severity). It never
-# creates bugs or attaches files.
+# Recordable action types the agent may take, by dotted id. A comment is the only one:
+# `bugzilla.update_bug` was here for `severity`, which is now a suggestion in the comment
+# instead, leaving the tool with no caller.
 #
-# `bugzilla.update_bug` needs `editbugs` on the apply account. The apply step coalesces
-# a same-bug field change with the nearest comment into one PUT, so losing that
-# privilege would take the analysis comment down with the rejected field change.
+# Dropping it also drops the `editbugs` requirement on the apply account, which mattered:
+# the apply step coalesces a same-bug field change with the nearest comment into one PUT,
+# so a rejected field change used to take the analysis comment down with it.
 ENABLED_ACTION_TYPES = [
     "bugzilla.add_comment",
-    "bugzilla.update_bug",
 ]
 
 
@@ -125,32 +123,21 @@ def key(self) -> str:
 # that `notify.py` keeps one flat mapping to look up.
 SLACK_CHANNELS = {c.key: c.channel for c in TRIAGE_SCOPE}
 
-# What a `bugzilla.update_bug` from this agent may touch. Enforced at record time
-# by `hooks.update_bug_hook`, so an out-of-bounds change is refused while the agent
-# can still correct it, rather than recorded and held for a human later.
-
-TRIAGE_FIELDS = frozenset({"keywords", "severity"})
-
 # Bugzilla's `bug_severity` legal values are `--`, `blocker`, `S1`, `critical`,
 # `S2`, `major`, `normal`, `S3`, `minor`, `S4`, `trivial`, `N/A`, `enhancement`
 # (https://bugzilla.mozilla.org/rest/field/bug/bug_severity). Narrowed to the four
 # `rules/severity-assessment.md` actually defines: the word forms are legacy, kept
 # for old bugs, and `--`/`N/A` mean unset or not-applicable, which is a metadata
 # regression rather than a triage judgment.
+#
+# The agent no longer writes the field; `agent.parse_severity` validates the level it
+# suggests against this set.
 TRIAGE_SEVERITIES = frozenset({"S1", "S2", "S3", "S4"})
 
-# Bugzilla defines ~340 keywords (https://bugzilla.mozilla.org/rest/field/bug/keywords,
-# or https://bugzilla.mozilla.org/describekeywords.cgi for the annotated list), several
-# of which drive automation. These six are the ones a frontend triage pass can add
-# without side effects. No ruleset in `rules/` directs a keyword addition today, so
-# widen this set alongside the rule that needs it rather than ahead of one.
-TRIAGE_KEYWORDS = frozenset(
-    {
-        "access",
-        "dataloss",
-        "good-first-bug",
-        "papercut",
-        "perf",
-        "regression",
-    }
-)
+# Which `severity_assessment.confidence` values are worth reporting. Below this the agent
+# says nothing about severity at all, since a level it is unsure of still reads as a
+# judgment an engineer may act on.
+#
+# `notify.py` reads this for the S1 marker; `rules/severity-assessment.md` repeats the
+# threshold for the comment block, because the model cannot import it. Change both.
+REPORTABLE_SEVERITY_CONFIDENCES = frozenset({"high", "medium"})

agents/frontend-triage/hackbot_agents/frontend_triage/hooks.py

@@ -2,117 +2,69 @@
 
 Once a run marks itself high-confidence its actions are applied to Bugzilla with no
 human in between, and an action's params are model output. These hooks bound that:
-one public comment and one add-only ``keywords``/``severity`` change, both on the
-bug being triaged.
+one public comment on the bug being triaged, and nothing else.
 
 They run at record time rather than at apply time for two reasons. The refusal
 reaches the agent as a tool error it can correct in the same run, and the
 out-of-bounds action never reaches ``summary.json``. ``ActionsRecorder`` runs hooks
 before appending, so raising here aborts the recording (see
 :data:`hackbot_runtime.actions.ActionHook`).
 
-The action **type** needs no check. ``ENABLED_ACTION_TYPES`` filters the tools the
-actions server exposes, so this agent has no way to record a
-``bugzilla.create_bug`` or a Phabricator action in the first place.
+The action **type** needs no check, and there is no field-change hook.
+``ENABLED_ACTION_TYPES`` filters the tools the actions server exposes, so this agent
+has no way to record a ``bugzilla.update_bug``, a ``bugzilla.create_bug`` or a
+Phabricator action in the first place -- ``severity`` is a suggestion in the comment
+now, which is what left that tool with no caller.
 """
 
 from __future__ import annotations
 
-from typing import Any
+import re
 
 from agent_tools.registry import ToolError
 from hackbot_runtime.actions import ActionHook, ActionsRecorder
 
-from .config import TRIAGE_FIELDS, TRIAGE_KEYWORDS, TRIAGE_SEVERITIES
+# A line the model writes to declare its severity. Anchored to the line start so an
+# ordinary mention -- quoting a reporter, or arguing why something is not S1 -- does
+# not count as a second declaration.
+_SEVERITY_DECLARATION = re.compile(r"^Suggested severity:", re.MULTILINE)
 
 
-def _check_only_one(recorder: ActionsRecorder, action_type: str) -> None:
-    # The rules ask for a single comment and at most one field change, but nothing
-    # else caps the count, and the agent reads every comment on the bug as untrusted
-    # input. A run told to write "a single brief comment" could record fifty.
-    if any(action["type"] == action_type for action in recorder.actions):
-        raise ToolError(
-            f"you have already recorded a {action_type}; record one per run, "
-            "revising it rather than adding another"
-        )
-
+def severity_block_hook(action: dict) -> None:
+    """Refuse a comment that declares its severity more than once.
 
-def _check_target_bug(params: dict, bug_id: int) -> None:
-    if params.get("bug_id") != bug_id:
+    The prompt asks for one block at the end. Nothing stops the model from also
+    writing the level into its analysis, and the reader would then have two
+    declarations that can disagree. Absence is fine and deliberate -- a run with low
+    severity confidence omits the block -- so this only fires on a second one.
+    """
+    text = (action.get("params") or {}).get("text")
+    if isinstance(text, str) and len(_SEVERITY_DECLARATION.findall(text)) > 1:
         raise ToolError(
-            f"you are triaging bug {bug_id}; record actions against that bug, "
-            f"not bug {params.get('bug_id')!r}"
+            "your comment declares a severity more than once; keep the single "
+            "`Suggested severity:` block at the end and drop the other"
         )
 
 
-def _check_severity(value: Any) -> None:
-    # Single-valued, so a scalar is the only way to set it — there is no additive
-    # form to insist on the way there is for keywords. `isinstance` before the
-    # membership test, because a list or dict is unhashable and `in` would raise
-    # rather than reject.
-    if not isinstance(value, str) or value not in TRIAGE_SEVERITIES:
+def _check_no_comment_yet(recorder: ActionsRecorder) -> None:
+    # The rules ask for a single comment, but nothing else caps the count, and the
+    # agent reads every comment on the bug as untrusted input. A run told to write
+    # "a single brief comment" could record fifty.
+    if any(action["type"] == "bugzilla.add_comment" for action in recorder.actions):
         raise ToolError(
-            f"severity {value!r} is not one you may set; "
-            f"use one of {', '.join(sorted(TRIAGE_SEVERITIES))}"
+            "you have already recorded a comment; record one per run, "
+            "revising it rather than adding another"
         )
 
 
-def _check_keywords(value: Any) -> None:
-    # A bare list *replaces* every keyword already on the bug; `{"add": [...]}` is
-    # the only form that adds one.
-    if not isinstance(value, dict) or set(value) != {"add"}:
-        raise ToolError(
-            'keywords must be added, not set: pass {"add": ["…"]} rather than '
-            f"{value!r}, which would replace the keywords already on the bug"
-        )
-    additions = value["add"]
-    if not isinstance(additions, list) or not additions:
-        raise ToolError("keywords' add must be a non-empty list")
-    # `isinstance` first: an unhashable entry would make `in` raise rather than reject.
-    unknown = [
-        k for k in additions if not isinstance(k, str) or k not in TRIAGE_KEYWORDS
-    ]
-    if unknown:
+def _check_target_bug(params: dict, bug_id: int) -> None:
+    if params.get("bug_id") != bug_id:
         raise ToolError(
-            f"keyword(s) {', '.join(repr(k) for k in unknown)} are not ones you may "
-            f"add; use one of {', '.join(sorted(TRIAGE_KEYWORDS))}"
+            f"you are triaging bug {bug_id}; record actions against that bug, "
+            f"not bug {params.get('bug_id')!r}"
         )
 
 
-def update_bug_hook(recorder: ActionsRecorder, bug_id: int) -> ActionHook:
-    """Refuse a ``bugzilla.update_bug`` outside what this agent is trusted with.
-
-    Mirrors what ``rules/frontend-triage.md`` and ``rules/severity-assessment.md``
-    sanction: one add-only change to fields in :data:`~.config.TRIAGE_FIELDS`, with
-    values from Bugzilla's own vocabulary, on ``bug_id`` — the bug the run was asked
-    about.
-    """
-
-    def hook(action: dict) -> None:
-        params = action.get("params") or {}
-        _check_only_one(recorder, "bugzilla.update_bug")
-        _check_target_bug(params, bug_id)
-
-        changes = params.get("changes")
-        if not isinstance(changes, dict) or not changes:
-            raise ToolError("changes must be a non-empty mapping of field to value")
-
-        disallowed = sorted(set(changes) - TRIAGE_FIELDS)
-        if disallowed:
-            raise ToolError(
-                f"you may not change {', '.join(disallowed)}; this agent changes only "
-                f"{', '.join(sorted(TRIAGE_FIELDS))}"
-            )
-
-        for field, value in changes.items():
-            if field == "severity":
-                _check_severity(value)
-            else:
-                _check_keywords(value)
-
-    return hook
-
-
 def add_comment_hook(recorder: ActionsRecorder, bug_id: int) -> ActionHook:
     """Refuse a ``bugzilla.add_comment`` this agent may not post.
 
@@ -123,7 +75,7 @@ def add_comment_hook(recorder: ActionsRecorder, bug_id: int) -> ActionHook:
 
     def hook(action: dict) -> None:
         params = action.get("params") or {}
-        _check_only_one(recorder, "bugzilla.add_comment")
+        _check_no_comment_yet(recorder)
         _check_target_bug(params, bug_id)
 
         if params.get("is_private"):