frontend-triage: load component guidance as needed - #6692
Draft
crythms wants to merge 7 commits into
Draft
Conversation
The system prompt carried where-the-code-lives notes for every area on every run -- about a third of its size -- and grew with each new component. A New Tab Page bug carried 10,300 characters of it to use 128. Each area now has a file under rules/areas/, and the prompt gets the one the bug's component maps to. That takes a recognized component from 32,313 characters to roughly 20,000, and keeps it flat as components are added: a new one costs a file and an index line instead of taxing every other run. Three things keep this from being less reliable than the prompt it replaces: - An unknown component, or a failed lookup, gets every area. rules/scoping.md puts an unlisted component in scope, so guessing one area would leave those runs with less than they have today. The lookup goes through the broker, since the agent container holds no Bugzilla credentials, and fails open rather than failing. - Firefox :: Sharing gets Site permissions alongside it. A "stop sharing" report arrives there but is WebRTC, which site permissions owns. - A comment citing code from an area the agent never loaded is refused, naming the area and the tool that fetches it. load_area_guidance shares the loaded set with the hook so the retry succeeds. A path in no area passes -- a bug that turns out to be Graphics must not block on guidance that does not exist. The prose moved verbatim, checked paragraph by paragraph against the original. Two cross-references changed because they pointed at blocks no longer in the same file, and headings are demoted on injection so guidance nests under Source repository rather than reading as a new top-level section.
The system prompt and the duplicate-detection ruleset both said the verdict never gates the triage, what the comment line looks like, and to fill in duplicate_assessment either way -- in different words, so the two could drift apart without either looking wrong. The system prompt keeps all three. It is read on every run, and the never-gate rule in particular is a safety property that cannot sit in a file the agent decides whether to open. Recording actions already says the stronger version of "you have no tool that changes a field", resolution included, so the ruleset was the third place that appeared. What is left in the ruleset is what a ruleset is for: what makes two reports the same defect, the per-area signals, and the bar for naming one at all.
…ely owns The guidance hook refused comments the guidance itself had asked for. Desktop frontend was listed as owning browser/, toolkit/ and devtools/, which contain most of the other areas, so an IP Protection bug citing browser/app/profile/firefox.js for its prefs -- a path rules/areas/ip-protection.md names -- came back refused. Six of eight realistic citations hit it: ordinary desktop chrome, an area's own test directory, a widget/ file from a bug that had nothing to do with sharing. Each one would have cost a run two turns recovering from a refusal it could not have avoided. Ownership is now separate from the index. `trees` stays descriptive and may overlap; `owns` is what the hook reads and has to mean "no other area could mean this file". Desktop frontend owns nothing, being the general case, and its guidance is the two lines it can least afford to lose. The narrow areas -- installer, Android, updater, IP Protection -- keep theirs, so the hook still fires where the guidance is worth enforcing. The regression test is per-component rather than per-area: every path an area's guidance names must survive the hook for every component that loads it, related areas included. That is what makes Sharing's reference to WebRTCParent legal, and it fails if the broad trees are ever marked owned again.
Four tests removed. Two restated the registry rather than testing behaviour, and two were covered by the per-component guidance-path invariant, which fails on the same defects -- checked by reintroducing each one and watching what broke. The docstrings said the same thing two or three ways. Kept the reasons that are not readable off the code: why the lookup happens before the run rather than at step 1, why `owns` is narrower than `trees`, why a path in no area passes, and why headings are demoted on the way into the prompt. Also two stale references the split left behind: render_scope still described the guidance as living under `Source repository`, and the AREAS comment still credited `trees` with driving the hook.
The lookup asked for `product,component`, and `get_bugs` diffs the ids it
requested against the ones it got back to report inaccessible bugs -- so leaving
`id` out of include_fields made the tool itself raise KeyError('id'). Every run
fell back to sending all eight areas.
It failed safely, which is the design working: a real run against Bugzilla still
produced a good triage, just with the prompt it had before this branch. But it
failed silently, because a tool-side error arrived here as a JSON parse failure
with the real message discarded. Check isError and put the message in the log.
Confirmed against a live broker: the lookup now resolves, and a New Tab Page bug
gets Desktop frontend alone.
`browser/components/contentsharing/` and `browser/modules/SharingUtils.sys.mjs` are both gone -- the tree is `browser/components/sharing/` now, with SharingUtils alongside ContentSharingUtils inside it. The names came from the prompt prose, which has been stale since before this branch (master names them too), and I took the ownership prefixes from that prose instead of checking the checkout. The effect was that Sharing owned nothing real, so the guidance hook could never fire for it. A live run on bug 2040869 localized into browser/components/sharing/content/content-sharing-modal.mjs and the hook stayed silent; those three paths resolve to Sharing now. The prose in rules/areas/sharing.md still names the old paths. That is the pre-existing bug and wants its own change -- this is only the ownership list.
The sharing tree was reorganised and the prompt was never updated -- these names have been wrong since before this branch. Everything is under browser/components/sharing/ now: browser/modules/SharingUtils.sys.mjs -> browser/components/sharing/ browser/components/contentsharing/ -> browser/components/sharing/ .../contentsharing/tests/browser/ -> .../sharing/tests/browser/ tests/unit/ (bare, ambiguous) -> .../sharing/tests/unit/ SharingUtils and ContentSharingUtils sit in one directory now rather than two, so the sentence introducing the second one says "alongside it" instead of naming a separate tree. Wording is otherwise untouched. Every path in the file now resolves against a current checkout. Checked the other seven area files the same way: the paths they name are relative-to-context fragments and all resolve.
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
1 participant
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.
Why
The system prompt described where the code lives for all eight areas — Android, the installer, IP Protection, and so on — on every run. A bug only ever needs one.
system.mdalso grew every time a component was added. The prompt has doubled in the past 7 weeks.What
Each area is now its own file under
rules/areas/. Before the run starts we look up the bug's component and put in just that area.The prompt no longer grows as components are added — a new one costs a file, not a tax on every other run.
Three things stop it being worse than today
3a. If the agent works out mid-run that the bug is in another area (component), it can't finish without reading that area. The comment is refused and told which one to load to continue triage.
3b. If there is no existing guidance to load — fall back to Searchfox as it does today
Tested on real bugs
Three runs against live Bugzilla, all of which produced a usable fix plan.
Quality held. 2065201 was also run with all eight areas, so the two are directly comparable: same three target files, same confidence, and the one-area run found more of the existing tests.
Two bugs turned up, both fixed here:
contentsharing/andbrowser/modules/SharingUtils.sys.mjsmoved intobrowser/components/sharing/, so nothing could ever match to Sharing. Those names have been wrong in the prompt since before this change; every path inrules/areas/is now checked against a real checkout.