Graphite Community

me too! was just about to post about it, but i guess it's not just me then!

Submission appears to be working again, cc <@U039TAB40D7> <@U02GFKHKJLA>

please let me know if it’s working for you as well

Of course! Apologies for the interruption - we’re updating Graphite to support on-prem GitHub installations, and that means refactoring all our syncing calls to GitHub to support dynamic hostnames. We got a callsite wrong here, and therefore had to rollback

&gt; we’re updating Graphite to support on-prem GitHub installations
:cat: :outbox_tray: :handbag: :question:

see <https://graphite-community.slack.com/archives/C02DRNRA9RA/p1676582901401379?thread_ts=1676582869.861839&amp;cid=C02DRNRA9RA>

Screen Shot 2023-02-16 at 5.57.08 PM.png

We are working on adding much more actionable messaging to this banner, and taking a look right now!!

It looks like what happened here is that one of your PRs was not passing all optional status checks, if you check the "ignore optional checks" box when merging, we'll skip those

Thank you! Will check on a diff stack soon and revert back

so I have to do `git rebase --continue` which breaks my flow

When I try `gt continue` after solving conflicts I get this
```ERROR: No Graphite operation to continue.```

```ERROR: Command failed with error exit code 1:
git rebase --onto master ee48cc9527b7a48ebe4d94bf6afdb4c32b1c64e4 AI-893
First, rewinding head to replay your work on top of it...```

the error seems to be coming from graphite cli?

Cc <@U02DGG322G5> another example of this bug

image.png

hi <@U04C4Q8GKB7>! You’re correct - this is a bug I accidentally introduced this morning

Rollback is already in flight, should disappear in a few minutes

:+1: its not blocking me in any way, just figured i would let you all know.

For context, we’re encouraging our internal team to dogfood staging. In the process, the encouraging button I added accidentally is visible to all users. We missed this in testing, because we were expecting to see it as employees

Can confirm the headers are gone now :+1:

Screen Shot 2023-02-17 at 12.21.54 PM.png

seems to be ok now! perhaps something on my side.

Thanks for flagging this - we'll look into it Glad everything's working now though

doesn't look like the network response has anything more verbose

hi <@U04G4UZ6TNV>, could you share (here or via DM) which PR is experiencing the AI summary error? we're working on adding better error messaging, and in the meantime I can check what error you were hitting

(~almost all errors today I see in our BE logs are due to PR size - we'll work on better exposing that limit to users)

Screenshot 2023-02-20 at 23.10.50.png

```│  │  ◉ 02-18-... (current) 
│  │  │ 15 minutes ago
│  │  │ 
│  │  │ PR #843 ...
│  │  │ <https://app.graphite.dev/github/pr/exponential-hq/isometric/843>
│  │  │ 
│  │  │ c1d6ca5 - ...
│  │  │
│  │  ◯ 02-20-... 
│  │  │ 16 minutes ago
│  │  │ 
│  │  │ PR #853 ...
│  │  │ <https://app.graphite.dev/github/pr/exponential-hq/isometric/853>
│  │  │ 
│  │  │ a9bef2d - ...
│  │  │
│  │  ◯ 02-20-... 
│  │  │ 16 minutes ago
│  │  │ 
│  │  │ PR #852 ...
│  │  │ <https://app.graphite.dev/github/pr/exponential-hq/isometric/852>
│  │  │ 
│  │  │ 7fd156c - ...
│  │  │
│  │  ◯ 02-20-... 
│  │  │ 27 minutes ago
│  │  │ 
│  │  │ PR #854 ...
|  |  | <https://app.graphite.dev/github/pr/exponential-hq/isometric/854>
│  │  │ 
│  │  │ c2774c2 - ...
│  │  │ df08afb - ...
│  │  │
├──┴──┘
◯ main 
│ 2 hours ago
│ 
│ b84eb6f - ...```

sorry – I am *almost* following the one commit per PR convention, with the exception for the bottom PR

I would have normally run `gt branch squash`'ed it, but now I'm a bit scared to do it (as something unexpected happened when I tried that a while ago as well)

to clarify – the local stack is exactly what I want, it's just that the topmost PR in the stack doesn't look the way it's supposed to on GitHub

i have been restacking the branches quite liberally – there was a bunch of disjoint work that I started to organise into a stack, where the PRs were originally created in an order that has nothing to do with the current order in the stack (as you can see by looking at the numbers)

finally, the 3 bottom PRs are fine in GitHub – they show up as `@kkom` wants to merge 2, 1 &amp; 1 commit(s) into the parent branch (respectively from the bottom)

which commit does PR #853's branch point to on github?

Screenshot 2023-02-21 at 08.47.09.png

i don't see any direct evidence for it, but it looks like `7fd156c` (which would be correct) – it has just one (correct) commit in the screenshot I'm attaching and the diff on GH checks out as well

same goes for the other 2 PRs

I'm going to have to start merging this stack to continue working, but I'll share some details on how the issue is evolving as I do this

Firstly, Graphite doesn't see any issues when doing `gt stack submit`:

```(backend-py3.11) ➜  backend git:(02-18-make_calculated_expression_tree_a_list) gt repo sync &amp;&amp; gt stack restack &amp;&amp; gt stack submit
🌲 Pulling main from remote...
main is up to date.
🧹 Checking if any branches have been merged/closed and can be deleted...
02-20-systematise_expression_variable_names_and_keys does not need to be restacked on main.
02-20-systematise_process_tests does not need to be restacked on 02-20-systematise_expression_variable_names_and_keys.
02-20-add_process-level_tests_to_test_key_uniqueness_and_separation does not need to be restacked on 02-20-systematise_process_tests.
02-18-make_calculated_expression_tree_a_list does not need to be restacked on 02-20-add_process-level_tests_to_test_key_uniqueness_and_separation.
🥞 Validating that this Graphite stack is ready to submit...

✏️  Preparing to submit PRs for the following branches...
▸ 02-20-systematise_expression_variable_names_and_keys (No-op)
▸ 02-20-systematise_process_tests (No-op)
▸ 02-20-add_process-level_tests_to_test_key_uniqueness_and_separation (No-op)
▸ 02-18-make_calculated_expression_tree_a_list (No-op)

🆗 All PRs up to date.
(backend-py3.11) ➜  backend git:(02-18-make_calculated_expression_tree_a_list) ```
(I won't redact branch names, they aren't that sensitive)

Merged the bottom one into main on GitHub and did this locally:

```(backend-py3.11) ➜  backend git:(02-18-make_calculated_expression_tree_a_list) gt repo sync &amp;&amp; gt stack restack &amp;&amp; gt stack submit
🌲 Pulling main from remote...
main fast-forwarded to 09957b5e074dc41b5895ca44dc98cdcb20d695f7.
🧹 Checking if any branches have been merged/closed and can be deleted...
✔ 02-20-systematise_expression_variable_names_and_keys is merged into main. Delete it? … yes
Set parent of 02-20-systematise_process_tests to main.
Deleted branch 02-20-systematise_expression_variable_names_and_keys
Restacked 02-20-systematise_process_tests on main.
Restacked 02-20-add_process-level_tests_to_test_key_uniqueness_and_separation on 02-20-systematise_process_tests.
Restacked 02-18-make_calculated_expression_tree_a_list on 02-20-add_process-level_tests_to_test_key_uniqueness_and_separation.
🥞 Validating that this Graphite stack is ready to submit...

✏️  Preparing to submit PRs for the following branches...
▸ 02-20-systematise_process_tests (Update)
▸ 02-20-add_process-level_tests_to_test_key_uniqueness_and_separation (Update)
▸ 02-18-make_calculated_expression_tree_a_list (Update)

📨 Pushing to remote and creating/updating PRs...
02-20-systematise_process_tests: <https://app.graphite.dev/github/pr/exponential-hq/isometric/852> (updated)
02-20-add_process-level_tests_to_test_key_uniqueness_and_separation: <https://app.graphite.dev/github/pr/exponential-hq/isometric/853> (updated)
02-18-make_calculated_expression_tree_a_list: <https://app.graphite.dev/github/pr/exponential-hq/isometric/843> (updated)
(backend-py3.11) ➜  backend git:(02-18-make_calculated_expression_tree_a_list) ```

Screenshot 2023-02-21 at 09.49.34.png

and now the top PR is fine – just 1 commit, instead of 3 of 5 (depending on what you expect would happen after the bottom PR with 2 commits was merged into main)

using `squash merge` strategy on `main` btw (always, it's the only allowed one in our monorepo)

and here is the `gt` version:

```(backend-py3.11) ➜  backend git:(02-18-make_calculated_expression_tree_a_list) gt --version
0.20.14```

Ha! This happened to me again – this time to a PR in the middle of a stack.

The stack looks like this:

• bottom PR (very new, just 1 commit locally and in GitHub)
• middle PR (very old – recently upstacked on the bottom one, just 1 commit locally, but incorrectly 2 commits in GitHub – the diff looks wrong)
• top PR (very old, has been stacked on top of the middle one for a long time, just 1 commit locally and in GitHub, the diff looks correct)

amending the commit in the middle PR and running `gt stack submit` fixed it - so at least the workaround is clear (and hopefully will be reliable)

2023-02-21 16.17.47.gif

Agree. Just about to merge a fix for this. Thanks for the feedback

<@U04ADV1CJ1M>, which PR count is off? The number of PRs queued to merge, or number merged in the past 7 days?

Ah, I see. I figured it was the total (which would have included the 11)

So next time you should see it go up to 14.

I have problems in general merging my stack. It always fails on the second job because we use squash and merge.

cc <@U02RREE9GKH> who is working on making the merging experience for stacks more reliable.

do you have an example of a PR that failed to merge that you expected to succeed?  we are working on adding better error messaging, but I can look up why specific jobs failed for now.

Sure, I'm trying right now. Those 11 PRs, the second failed.

How can I help diagnose. I still have 10 to merge

Interesting error! I haven't seen this one before —

```Command failed: git push -f origin am/replace-transactions-w-journals\nremote: error: GH007: Your push would publish a private email address.        \nremote: You can make your email public or disable this protection by visiting:        \nremote: <http://github.com/settings/emails>        \nTo <https://github.com/FinTronners/FTron>\n ! [remote rejected] am/replace-transactions-w-journals -&gt; am/replace-transactions-w-journals (push declined due to email privacy restrictions)\nerror: failed to push some refs to '<https://github.com/FinTronners/FTron>'\n"```

But yeah, we will be surfacing errors like this more clearly in the coming weeks.

If it helps, after every merge I get this notification from Github:

That is because of GitHub not knowing about stacks first class. You should be able to fix the error that Jacob linked above by unchecking attached setting in `<http://github.com/settings/emails|github.com/settings/emails>`.

<@U02RREE9GKH>, i wonder if we can support users who have this setting checked?

Re the merge conflicts message — those merge conflicts are resolved by a rebase, which the graphite merge performs automatically for you.  So that notification is nothing to worry about.

That's what I figured. I assumed it was due to the squash and merge behavior

Also getting this message now:

Hope I'm not being spammy. I just love the app and trying to help.

Not at all! I'm actively working on this, so it's quite helpful

Hi, I'm sorry you are running into this. It looks like we had a change today that is causing some sporadic merge failures. I'm reverting the offending changes now.

The revert has been deployed, please let me know if you continue to see issues.

Screenshot 2023-02-22 at 4.20.20 PM.png

I’m seeing these a lot today, but they’re not actually blocking merging, so they’re just weird and annoying.

the href seems to match in both — the diff I see is the `target="_blank"` which should open the link in a new tab

The Graphite markup includes an additional `&lt;a href&gt;` tag, so the button is "double-wrapped".  The _outer_ tag has the correct href, but the _inner_ tag has an href that actually points to the image uri.
When you click the button, the browser navigates to the actual button image rather than the original href.

we're doing something really wonky, it looks like :melting_face:

i see some Object.toString() in there as well...

:+1: Thanks for taking a look!

I've been using the graphite web ui for a week now instead of github's ui and it's been great!

anyway, thanks for reporting! We'll dig in

Badge-links appear to be working in PRs that I opened today!
Thanks, team!

I see a huge discrepancy as well between github "review requested" and Graphite "needs your review" today

Are they in the timeline view at the right?

Ah! They are! Found that really hard to see to be honest :sweat_smile: thanks!

No problem! We have some improvements planned there cc <@U02DCQ1C0G6>

I’m currently looking into this issue. It looks like the merge is successful, but the comment is incorrect. 

I’ll take a look, thanks for reporting!

Thanks guys, love the promptness on this stuff :pray:

Not familiar with Vue - do these files have `.vue` extensions or regular `.js` extensions? It might be hard to autodetect file types if it’s the latter, but I’ll still look into adding a Vue specific syntax highlighting scheme

`.vue` files should be highlighted now, LMK if there are any issues!

The detected filetype might still say plain text - that’s a known issue which I intend to fix in the coming week :smile:

Just checked it out - this is great, thank you so much!

Thanks for the feedback and happy that helped!

A very minor thing, in suggestions now it still thinks `vue` is text! (super minor though)

Mmm, I honestly completely missed that surface. I’ll look in to it, hopefully in the next few days :slightly_smiling_face: Thanks for these bug reports/feature requests, these are super helpful and please keep them coming!

Literally no rush on this one, just noticed it is all!

Thank you for flagging! The problem is we don't currently check branch protection rules in real time on the PR page. When you refresh, we recalculate this which is what you're seeing. This is something I'm actively working on this quarter and hope to make improvements here soon

:raised_hands:  awesome (again, not a big deal)

We're working on fixing the error messaging here - in most (if not all) failure cases the diff is too big

What are the thresholds for too big? And is it just diff size or is the size of the file modified also a factor (irrespective of diff size)?

only diff size: unchanged lines in a file aren't counted. as for threshold, right now I'd say diffs that exceed a very rough ballpark of 100 lines are more likely to hit the cap, especially if the lines themselves are long

I plan to look more closely at failure cases to identify a clearer line to communicate in the app. the root cap is from openai token limits, which is proportional but not identical to character count. we compress the diff itself by stripping out or simplifying lines that we expect to not impact the summarization quality, so there's high variance in indicating a cap by just line or character count alone.

copy tyvm!

100 lines is pretty tiny! but good to know so i know i can try it on something small

Hi Ben, I'm sorry you are having this issue. I'll take a look at what is going on right now.

I see the issue and am working on a fix. I'll let you know once it is deployed.

The fix for this should now be deployed. Please let me know if you encounter any more issues.

Just checked - it's working after a refresh. Thanks again!

Editing is still working great, but I have seen the 'save' button not get activated during editing a few times this morning. `ctrl+enter` still works.

I believe a fix we landed yesterday should resolve this — when was the last time you noticed it?

next time it happens, can you let me know which PR + version number (of the PR) it was on?

Hi, ok so I just clicked into a PR just now (first time loading Graphite since last Friday, so everything should be fresh?) and I see at the top of the page there’s a gray banner that says “This pull request was updated while you were viewing it”. I’m fairly certain that this isn’t the case? GitHub says the PR was last updated 1 hour ago

the PR is `sym-flow-cli#207` (it’s a private repo tho). Graphite opened on version 3, but looks like version 4 is the most recent

Hmm, that is the approach we took to fix this bug, but it shouldn't be showing up in that case — seems like we might be caching too much. i'll take a look!

hey <@U04R4R6GGVC>! before i take a look at your PR wanted to quickly check to make sure you did a `gt stack restack` and resubmitted the PR if you made any changes to the big change - sorry if this is super obvious

usually changes from a previous PR in a stack appear in a PR just upstack of that one if a `restack` and `submit` are not done after making changes

That does make sense, I think in this instance I’d had both the pr’s, clicked merge on the top one (2058) and once the bottom one had merged the diff between the top one and main was the combination of both prs which was strange. Maybe that’s bc I should have done it individually and run gt stack restack between each merge?

<@U04R4R6GGVC> we generally recommend merging from bottom to top.

So let's say you have main -&gt; branch A -&gt; branch B, I would either:
1. Merge as stack from Graphite UI
2. Merge from the bottom, i.e., branch A to main first then run `gt rs -r &amp;&amp; gt ss` and then merge branch B


Hmm, I guess in my head that should “just work” when I click the `Merge Stack of 6` button. Is it not possible for Graphite to run the `gt stack restack` command on the server side instead of me doing it manually everytime? I ask because that’s pretty much the same flow I would have to do for github anyways

Definitely aggressive question, trying to figure out where/how this tool works for my use case so definitely understand if this isn’t a covered use case

Yes, the `Merge stack of 6` button should just work and merge all (option 1 in my message above). So to be clear, this button did not work for you, correct? Do you have a merge job number so we can look into why it failed? cc: <@U02RREE9GKH>

Yeah this was the link to the PR, I think the job id is 2052? <https://app.graphite.dev/github/pr/withbroadcast/broadcast/2052/Searching-for-Linear-Users>

I made a thread here about it <https://graphite-community.slack.com/archives/C02ENKC0A9X/p1677443950800309>

Generally the merge stack of 6 has worked pretty nicely for me for the last few months I’ve only noticed it breaking pretty frequently this last week. Definitely loving the tool so was confused/assumed I must have done something wrong in the set up of the branches or something

Hi! we pushed a change last night that should fix this issue - i.e. perform the rebase correctly on our end :slightly_smiling_face:

Wooo thank ya I’ll give it a try soon, I’m building another 6+ diff stack right now (we love shifting deadlines and PRDs :man-facepalming:) so should be able to test it again? I’ll let yall know if I run into another issue

This is the big change <https://app.graphite.dev/github/pr/withbroadcast/broadcast/2055/Linear-Cycle-Filter>

It's been a few days, I thought it might be a glitch so I didn't report it right away

We've temporarily unshipped this - we'll let you know when it's back!

For context, we realized that the implementation of this feature was causing slowdowns in code load on the PR page — we're working on fetching it from GitHub in such a way to prevent this in the next iteration!

super clear, thanks! Maybe it could be a user triggered request, eg a button next to the "viewed" checkbox? Something like a "show blame" button ?

We have considered this, but have not landed on the long-term correct behavior here because currently the story around syncing that rebase back to local via the CLI would be somewhat unclear.  Once the CLI can sync these updates for you from remote and handle the case where they've changed locally reliably we may revisit this.

Hi Oleg! What you're noticing here is that we only rebase the PR if it needs to be rebased before merging.  To explain further — if the same file is changed in consecutive PRs, they will need to be rebased as they merge, otherwise they won't (due to merge and rebase having slightly different methods of detecting a conflict).  So what you're noticing here might be that in this stack the PRs didn't need to be rebase, whereas in some other they did.

Rebasing each PR in the stack if not necessary would result in slower merges, especially for our users with longer CI runs (or even worse, flaky CI checks). We're considering a toggle in the merge stack that allows this to be chosen at queue time.

Hey Jacob, thanks for explaining! I guess the reason you see the bigger diff in the github UX is that when the automatic base change happens, it's to the base that existed prior to the merge and not the latest one. I understand this is an optimization but wonder if Graphite could work around that and somehow show a cleaner diff?

Also as far as improving CI runs merge queues could help there <https://github.blog/changelog/2023-02-08-pull-request-merge-queue-public-beta/>

Otherwise I'd support a toggle to force rebase all branches in the stack

&gt; We're considering a toggle in the merge stack that allows this to be chosen at queue time.
That would be amazing. I'd even go as far as make it an org-wide setting with no possibility of deviating from it (if an admin chooses to). Similar to how you may want to enforce that the only legal merge strategy is "squash + merge" (which we do).

Exactly what I've been thinking! org- and repo-level settings like this will be part of our upcoming merge queue offering (the "queue" part is optional :wink: )— if you're interested in getting your team signed up as a design partner, I would recommend chatting with <@U042CEGKRKM>

Thanks Jacob! On queues, frankly the GH version of merge queues being in GA makes me lean towards it (tight coupling to CI/CD or branch protection, already paid for, obviously works for team members not using Graphite — and we’re on monorepo so even more important, etc). GH has had its stability issues too, but again frankly — I see more stability issues in Graphite than in GitHub lately (which is expected — you ship more and you should!)

But still very interested to have a chat about what a Graphite-powered merge queue would mean for us — because some of our assumptions may be wrong.

Cool to see the option appear in the UX recently, great work!

I think <@U047KHH8JH3> was looking into bugs on this endpoint! Sorry you're hitting this

&gt; I also see credential failure when I do gt ss on a new stack, however it appears to still make it to GitHub
What's your github login?

Thanks for the response <@U02RREE9GKH>, Are you looking for email used with GitHub?

It is <mailto:wraeth.dao@gmail.com|wraeth.dao@gmail.com>

Hi Jordan, mind checking your permissions in Account Settings -&gt; Manage GitHub Access? It seems like there may be an issue with your GH auth

That page looks right. Some notes:
• It's set up with the Github apps integration
• We previously set up with OAuth and revoked that permission

Had more issues with this today, respos stopped syncing and the github app disappeared in my account. Had to go re-add it, and now my dashboard is showing old PR statuses (e.g. showing open PRs when they have been closed). If I go and open the PR the state gets corrected, but I need to manually do it for everything in my dashboard. It also looks like there's quite a few PRs missing