https://llmservingsim.ai/assets/js/0841e251.998a686f.js

1"use strict";(self.webpackChunkwebsite=self.webpackChunkwebsite||[]).push([["6102"],{64908(e,n,i){i.r(n),i.d(n,{metadata:()=>s,default:()=>d,frontMatter:()=>o,contentTitle:()=>h,toc:()=>l,assets:()=>a});var s=JSON.parse('{"id":"contributor/pr-workflow","title":"PR workflow","description":"How a contribution gets from your fork into main. Read this once;","source":"@site/docs/contributor/pr-workflow.md","sourceDirName":"contributor","slug":"/contributor/pr-workflow","permalink":"/docs/contributor/pr-workflow","draft":false,"unlisted":false,"editUrl":"https://github.com/casys-kaist/LLMServingSim/tree/main/website/docs/contributor/pr-workflow.md","tags":[],"version":"current","sidebarPosition":6,"frontMatter":{"sidebar_position":6,"title":"PR workflow"},"sidebar":"contributorSidebar","previous":{"title":"Validating your changes","permalink":"/docs/contributor/validating-changes"}}'),r=i(74934),t=i(31137);let o={sidebar_position:6,title:"PR workflow"},h="PR workflow",a={},l=[{value:"Branch model",id:"branch-model",level:2},{value:"Commit hygiene",id:"commit-hygiene",level:2},{value:"Before you push",id:"before-you-push",level:2},{value:"Opening the PR",id:"opening-the-pr",level:2},{value:"What review looks like",id:"what-review-looks-like",level:2},{value:"Squash, rebase, or merge?",id:"squash-rebase-or-merge",level:2},{value:"Attribution",id:"attribution",level:2},{value:"After merge",id:"after-merge",level:2},{value:"What's next",id:"whats-next",level:2}];function c(e){let n={a:"a",code:"code",em:"em",h1:"h1",h2:"h2",header:"header",li:"li",ol:"ol",p:"p",pre:"pre",strong:"strong",ul:"ul",...(0,t.R)(),...e.components};return(0,r.jsxs)(r.Fragment,{children:[(0,r.jsx)(n.header,{children:(0,r.jsx)(n.h1,{id:"pr-workflow",children:"PR workflow"})}),"\n",(0,r.jsxs)(n.p,{children:["How a contribution gets from your fork into ",(0,r.jsx)(n.code,{children:"main"}),". Read this once;\nafterward each PR should take you about ten minutes of process\noverhead on top of the actual work."]}),"\n",(0,r.jsx)(n.h2,{id:"branch-model",children:"Branch model"}),"\n",(0,r.jsxs)(n.ul,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:(0,r.jsx)(n.code,{children:"main"})}),": the active development branch. All PRs land here."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Artifact branches"}),": per-paper reproducibility branches, named\nafter the venue (e.g., ",(0,r.jsx)(n.code,{children:"ispass26-artifact"}),"). ",(0,r.jsx)(n.strong,{children:"Don't open PRs\nagainst these."})," They are frozen at the artifact submission state."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Your work"}),": a feature branch off ",(0,r.jsx)(n.code,{children:"main"}),", named descriptively\n(",(0,r.jsx)(n.code,{children:"add-deepseek-v3"}),", ",(0,r.jsx)(n.code,{children:"fix-evict-accumulation"}),", ",(0,r.jsx)(n.code,{children:"docs-cluster-config"}),").\nDon't push to ",(0,r.jsx)(n.code,{children:"main"})," directly even if you have permissions."]}),"\n"]}),"\n",(0,r.jsx)(n.pre,{children:(0,r.jsx)(n.code,{className:"language-bash",children:"git checkout main\ngit pull\ngit checkout -b add-deepseek-v3\n"})}),"\n",(0,r.jsx)(n.h2,{id:"commit-hygiene",children:"Commit hygiene"}),"\n",(0,r.jsxs)(n.ul,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Short imperative one-liner."})," Same style as the existing log:\n",(0,r.jsx)(n.code,{children:"Fix incorrect evict_size accumulation"}),", ",(0,r.jsx)(n.code,{children:"Add Qwen3 model support"}),", ",(0,r.jsx)(n.code,{children:"Document MoE expert routing"}),"."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"One logical change per commit."})," A refactor and a feature in\nthe same commit is a reviewer's nightmare."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Don't amend published commits."})," If you pushed it, follow up\nwith a new commit. Force-pushing your branch is fine ",(0,r.jsx)(n.em,{children:"before"}),"\nreview starts, generally not after."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"There are no pre-commit hooks"})," in this repo, so nothing runs\nautomatically on commit and ",(0,r.jsx)(n.code,{children:"--no-verify"})," has nothing to bypass. The\nchecks below are yours to run."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsxs)(n.strong,{children:["No ",(0,r.jsx)(n.code,{children:"Co-authored-by"})]})," unless someone really did pair-program\nwith you on this commit."]}),"\n"]}),"\n",(0,r.jsx)(n.p,{children:"A good commit message:"}),"\n",(0,r.jsx)(n.pre,{children:(0,r.jsx)(n.code,{children:"Fix evict_size accumulation when prefix cache spills to CPU\n\nSpilling counted the block twice: once in the NPU eviction and\nagain when the second-tier pool inserted it. Drop the second\nincrement; the test in single_node_memory_instance.json now\nmatches the bench reference.\n"})}),"\n",(0,r.jsx)(n.p,{children:"A bad one:"}),"\n",(0,r.jsx)(n.pre,{children:(0,r.jsx)(n.code,{children:"fixes\n"})}),"\n",(0,r.jsx)(n.h2,{id:"before-you-push",children:"Before you push"}),"\n",(0,r.jsx)(n.p,{children:"Run through the checklist:"}),"\n",(0,r.jsxs)(n.ol,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Smoke run passes."})," See ",(0,r.jsx)(n.strong,{children:(0,r.jsx)(n.a,{href:"./validating-changes",children:"Validating your\nchanges"})}),", step 1."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Targeted scenarios pass"})," for whatever you touched. Step 2."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Bench validation hasn't regressed"})," if your change affects\nend-to-end accuracy. Step 3."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Conventions checklist"}),": ",(0,r.jsx)(n.code,{children:"getattr"})," fallbacks, ",(0,r.jsx)(n.code,{children:"head_dim"}),"\nhandling, English-only, layer names, no ",(0,r.jsx)(n.code,{children:"astra-sim/inputs

1/"}),"\nedits. See ",(0,r.jsx)(n.strong,{children:(0,r.jsx)(n.a,{href:"./conventions",children:"Coding conventions"})}),"."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Docs updated"})," if behavior changed. The relevant page under\n",(0,r.jsx)(n.code,{children:"docs/"}),", plus the module's ",(0,r.jsx)(n.code,{children:"README.md"})," if applicable."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"No machine-specific paths or generated files"})," in the diff.\nSanity-check with ",(0,r.jsx)(n.code,{children:"git diff --stat"})," and\n",(0,r.jsx)(n.code,{children:"git diff --check"}),"."]}),"\n"]}),"\n",(0,r.jsx)(n.h2,{id:"opening-the-pr",children:"Opening the PR"}),"\n",(0,r.jsx)(n.p,{children:"Push to your fork (or branch if you have direct access):"}),"\n",(0,r.jsx)(n.pre,{children:(0,r.jsx)(n.code,{className:"language-bash",children:"git push -u origin add-deepseek-v3\n"})}),"\n",(0,r.jsxs)(n.p,{children:["Then open the PR against ",(0,r.jsx)(n.code,{children:"casys-kaist/LLMServingSim:main"}),". The\ndescription should include:"]}),"\n",(0,r.jsx)(n.pre,{children:(0,r.jsx)(n.code,{children:"## What this changes\n\nA 1-3 sentence summary of the user-visible change.\n\n## Why\n\nThe motivation: the bug it fixes, the feature it enables, the\nresearch question it lets you ask.\n\n## Validation\n\nThe exact command(s) you ran and the key result. For example:\n\n  ./bench/examples/validate.sh RTXPRO6000/Llama-3.1-8B\n  -> TTFT MAPE 2.1% (was 2.3%), TPOT 1.7% (unchanged)\n\n## Notes\n\nAnything subtle: known limitations, related issues, follow-ups\nyou intentionally did not include.\n"})}),"\n",(0,r.jsx)(n.p,{children:"You don't need a heavy template. The validation section is the one\nnon-negotiable part: it gives the reviewer something concrete to\nrerun and gives the git log a record of what was checked."}),"\n",(0,r.jsx)(n.h2,{id:"what-review-looks-like",children:"What review looks like"}),"\n",(0,r.jsxs)(n.ul,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Initial response"}),": usually within 2-3 days for the first round.\nTime-zone overlap with KAIST (UTC+9) helps but isn't required."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Reviewers"}),": at least one of the main contributors\n(",(0,r.jsx)(n.a,{href:"https://github.com/JaehongCho",children:"@JaehongCho"}),",\n",(0,r.jsx)(n.a,{href:"https://github.com/hmchoi",children:"@hmchoi"}),") plus whoever owns the touched\narea. For docs-only PRs, one approval is enough."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"What gets blocked vs. nit-picked"}),":","\n",(0,r.jsxs)(n.ul,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Blockers"}),": an unexplained ",(0,r.jsx)(n.code,{children:"./serving/validate.sh"}),' difference,\nbench regressions beyond ~5%, convention violations from the\n"never do this" list, missing docs for new flags.']}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Nits"}),': naming, code style preferences, doc phrasing. The\nreviewer will say "nit:" or use the GitHub label. Address them\nif you agree; defer with a sentence if you don\'t.']}),"\n"]}),"\n"]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Conversation style"}),": terse and direct. \"This won't work for\nMoE\" is not a personal attack; it's faster than the polite\nversion. Reply in kind."]}),"\n"]}),"\n",(0,r.jsx)(n.h2,{id:"squash-rebase-or-merge",children:"Squash, rebase, or merge?"}),"\n",(0,r.jsxs)(n.p,{children:["The project squashes most PRs to a single commit on ",(0,r.jsx)(n.code,{children:"main"}),", with\nthe PR title becoming the commit message. You don't need to clean\nup your branch's intermediate commits beforehand. If your PR is\ngenuinely best as multiple commits (e.g., a refactor + a feature\nthat depends on it), say so in the description and a maintainer\nwill rebase rather than squash."]}),"\n",(0,r.jsx)(n.h2,{id:"attribution",children:"Attribution"}),"\n",(0,r.jsx)(n.p,{children:"External contributors get credit in two places:"}),"\n",(0,r.jsxs)(n.ol,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"GitHub commit history"}),": your authorship is preserved on\nmerge."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:(0,r.jsx)(n.code,{children:"CONTRIBUTORS.md"})}),': the maintainer adds a line with your\nGitHub handle and a link to the PR or issue \u2014 under\n"C

1ode" for a merged patch, under "Reports and analysis" for an\nissue that pinned down a real problem. Reports get their own\nsection rather than a footnote, and the changelog entry for the\nfix names you too.']}),"\n"]}),"\n",(0,r.jsx)(n.p,{children:"You don't need to add yourself to the contributors list in your\nPR. The maintainer adds it on merge."}),"\n",(0,r.jsx)(n.h2,{id:"after-merge",children:"After merge"}),"\n",(0,r.jsxs)(n.ul,{children:["\n",(0,r.jsxs)(n.li,{children:[(0,r.jsxs)(n.strong,{children:["Pull ",(0,r.jsx)(n.code,{children:"main"})]})," before starting the next change. Your local\nbranch is no longer authoritative."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"Delete the merged branch"})," locally and on the remote\n(GitHub offers a button after merge; ",(0,r.jsx)(n.code,{children:"git branch -d add-deepseek-v3"}),"\nlocally)."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsxs)(n.strong,{children:["Re-run your scenarios against ",(0,r.jsx)(n.code,{children:"main"})," after the merge."]})," There is\nno test CI to watch: the only workflow is ",(0,r.jsx)(n.code,{children:"deploy-docs.yml"}),", which\nbuilds the docs site and says nothing about the simulator. If\nsomething broke that the review missed, the way you find out is by\nrunning ",(0,r.jsx)(n.code,{children:"./serving/validate.sh"})," on ",(0,r.jsx)(n.code,{children:"main"}),"."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"My PR sat for a week with no reviews."})," Ping the PR with a\none-liner. Maintainers do miss notifications."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"A reviewer requested changes I disagree with."})," Explain your\nreasoning in a comment. If you still disagree after the\nreviewer's reply, escalate by tagging the other main contributor\nfor a tiebreaker. We'd rather have the discussion than land the\nwrong design."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsx)(n.strong,{children:"My change regressed bench beyond what I expected."})," Don't merge\nit. Open the PR as a draft and tag the regression in the\ndescription; we'll figure out together whether it's a bug in your\nchange, in the existing baseline, or in the validation\nmethodology."]}),"\n",(0,r.jsxs)(n.li,{children:[(0,r.jsxs)(n.strong,{children:["I broke something on ",(0,r.jsx)(n.code,{children:"main"}),"."]})," It happens. Open a follow-up PR\nwith a ",(0,r.jsx)(n.code,{children:"Fix ..."})," commit; don't ",(0,r.jsx)(n.code,{children:"git push --force"})," to ",(0,r.jsx)(n.code,{children:"main"}),"."]}),"\n"]}),"\n",(0,r.jsx)(n.h2,{id:"whats-next",children:"What's next"}),"\n",(0,r.jsxs)(n.p,{children:["You've got the full picture now. Go pick a starter issue or open\na new one with ",(0,r.jsx)(n.code,{children:"[contributor]"})," in the title to discuss what you'd\nlike to work on."]}),"\n",(0,r.jsx)(n.p,{children:"Welcome aboard."})]})}function d(e={}){let{wrapper:n}={...(0,t.R)(),...e.components};return n?(0,r.jsx)(n,{...e,children:(0,r.jsx)(c,{...e})}):c(e)}},31137(e,n,i){i.d(n,{R:()=>o,x:()=>h});var s=i(2086);let r={},t=s.createContext(r);function o(e){let n=s.useContext(t);return s.useMemo(function(){return"function"==typeof e?e(n):{...n,...e}},[n,e])}function h(e){let n;return n=e.disableParentContext?"function"==typeof e.components?e.components(r):e.components||r:o(e.components),s.createElement(t.Provider,{value:n},e.children)}}}]);